LINUX.ORG.RU
Форум — Development  

покритикуйте код


0

0 

покритикуйте код. Можно ли его сделать проще/читабельнее?

void NMSPort::shutdown_handler(const Engine::Event& evt)
{
    // try to disconnect if line is active
    NCC_LINE_STATUS line_status;
    char *message = NULL;

    DWORD retcode =
        nccGetLineStatus(ctx, &line_status, sizeof(line_status), NULL, 0);
    if(retcode == SUCCESS)
    {
        if(line_status.state == NCC_LINESTATE_ACTIVE)
        {
            retcode = nccDisconnectCall(_callh, NULL);
            if(retcode == SUCCESS)
            {
                Engine::Event evt(Engine::Event::EVT_CC_CALL_DISCONNECTED);

                if(wait_for_specific_event(evt, ENG_TIMEOUT))
                {
                    if(evt.id() == Engine::Event::EVT_ENG_TIMEOUT)
                    {
                        message = "shutdown_handler: wait_for_specific_event() timeout";
                    }
                    else
                    {
                        retcode = nccReleaseCall(_callh, NULL);
                        if(retcode == SUCCESS)
                        {
                            evt = Engine::Event(Engine::Event::EVT_CC_CALL_RELEASED);
                        
                            if(wait_for_specific_event(evt, ENG_TIMEOUT))
                            {
                                if(evt.id() == Engine::Event::EVT_ENG_TIMEOUT)
                                {
                                    message = "shutdown_handler: wait_for_specific_event() timeout";
                                }
                            }
                            else
                            {
                                message = "shutdown_handler: wait_for_specific_event() failed";
                            }
                        }
                        else
                        {
                            message = "shutdown_handler: nccReleaseCall() failed";
                        }
                    }                    
                }
                else
                {
                    message = "shutdown_handler: wait_for_specific_event() failed";
                }
            }
            else
            {
                message = "shutdown_handler: nccDisconnectCall() failed";
            }
        }
    }
    else
    {
        message = "shutdown_handler: nccGetLineStatus() failed";
    }

    if(message)
        show_error(message);
}
anonymous

Можно, exception'ы используй.

imp ★★
() 

Это какой то трындец.

void NMSPort::shutdown_handler(const Engine::Event& evt)
{
    // try to disconnect if line is active
    NCC_LINE_STATUS line_status;
    char *message = NULL;

    DWORD retcode =
        nccGetLineStatus(ctx, &line_status, sizeof(line_status), NULL, 0);
    if(retcode != SUCCESS)
    {
        message = "shutdown_handler: nccGetLineStatus() failed";
        goto fini;
    }

    if(line_status.state != NCC_LINESTATE_ACTIVE) {
        // ???
    }

    retcode = nccDisconnectCall(_callh, NULL);
    if(retcode != SUCCESS) {
        message = "shutdown_handler: nccDisconnectCall() failed";
        goto fini;
    }
    ...

fini:
    if(message)
        show_error(message);
}

Ну а вообще да, лучше использовать исключения.

Legioner ★★★★★
()

>const Engine::Event& evt
>Engine::Event evt(Engine::Event::EVT_CC_CALL_DISCONNECTED);
>evt = Engine::Event(Engine::Event::EVT_CC_CALL_RELEASED);

Меня почему-то эти строчки нервируют.

>message = "shutdown_handler: wait_for_specific_event() timeout";

Это нормально, что оно из разных мест вылазит? Имеются телепаты или я чего-то об Engine и wait_for_specific_event() не знаю?

redgremlin ★★★★★
() 

Я бы сделал примерно так:

func() {
  
  if (!stmt1) {
    show_error(err_msg1);
    return;
  }

  if (!stmt2) {
    show_errror(err_msg2);
    return;
  }

  ...
}

php-coder ★★★★★
()

всем спасибо. С goto вариант больше понравился.

А с исключениями этот код, если грамотно сделать, как будет выглядеть?

anonymous
()
Ответ на: комментарий от anonymous

>А с исключениями этот код, если грамотно сделать, как будет выглядеть?

try {
if (st1) throw errmsg1;
if (st2) throw errmsg2;
if (st3) throw errmsg3;
} catch (char* str) {
show_error(str);
}

redgremlin ★★★★★
() 

#define SURE(x)  \
if (!(x)) \ 
{ \
   message= #x; \
   /* can also get __FILE__:__LINE__ here */ \
   break; \
}

и потом 

do {
  SURE(...);
  SURE(...);
  ...
} while (0);

gods-little-toy ★★★
()
Ответ на: комментарий от UnDeFiNeD

>goto - это плохая идея.

exceptions еще хуже!! достаточно посмотреть на листинги, что делает try-catch

правда, goto не C++-но.

xydo ★★
() 

> void NMSPort::shutdown_handler(const Engine::Event& evt)

сделай ее 

const char * NMSPort::shutdown_handler(const Engine::Event& evt)

и далее 

{
  ...;

  if(..!=..)
    return "....";

  ...;

  if(..!=..)
    return "....";

  ...;

  if(..!=..)
    return "....";

  ...;
  return 0;
}

а show_error(message) помести во внешнюю функцию

xydo ★★
()
Ответ на: комментарий от UnDeFiNeD

> нет, goto - это плохая идея. Не стоит им пользоваться.

В литературе часто оговаривают это конкретный случай (обработка ошибок) как удачный способ применения goto.

anonymous
()

можно, почитай про RAII

anonymous
()
Ответ на: комментарий от gods-little-toy

собстно goto "запретили" потому что при неумелом использовании порождается страшнейший и труднодебажный код. Поэтому во многих языках goto unimplemented по этой причине.

А так, в ядре и некоторых частных случаях более чем уместно.

true_admin ★★★★★
()
Ответ на: комментарий от true_admin

да, причем, как показывает практика, такой код можно намутить не только с помощью goto)))

xydo ★★
()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.
Development