Красота кода
От: iix  
Дата: 31.01.06 18:33
Оценка:
Всем привет.
Вот сижу и программирую клиента для одного IM и в одной функции получился извращенный до нельзя код. Хотелось бы чтобы вы предложиле более красивую реализацию данного кода:

    unsigned int thrdaddr;
    HANDLE mthrd[2];

    if(connected && !logining)
    {
        if(sendmtx=CreateMutex(NULL,false,NULL))
        {
            if(recvmtx=CreateMutex(NULL,false,NULL))
            {
                if(pingthrd=(HANDLE)_beginthreadex(NULL,0,(PTHREAD)PingThread,this,0,&thrdaddr))
                {
                    if(recvthrd=(HANDLE)_beginthreadex(NULL,0,(PTHREAD)RecvThread,this,0,&thrdaddr))
                    {
                        DbgPrint("Login2: succesfull...");
                        logining=true;
                        return true;
                    }
                    else
                        DbgPrint("Login2: hello error...");
                }
                else
                    pingthrd=(HANDLE)_beginthreadex(NULL,0,(PTHREAD)PingThread,this,0,&thrdaddr);

            }
            else
                DbgPrint("Login2: recv CreateMutex error...");
        }
        else
            DbgPrint("Login2: send CreateMutex error...");
    }

    TerminateThread(pingthrd,0);
    TerminateThread(recvthrd,0);
    mthrd[0]=pingthrd;
    mthrd[1]=recvthrd;
    WaitForMultipleObjects(2,mthrd,true,INFINITE);
    CloseHandle(pingthrd);
    CloseHandle(recvthrd);

    CloseHandle(sendmtx);
    CloseHandle(recvmtx);

    return false;



01.02.06 18:13: Перенесено из 'Философия программирования'
Re: Красота кода
От: GlebZ Россия  
Дата: 31.01.06 18:42
Оценка: +1
Здравствуйте, iix, Вы писали:

iix>Всем привет.
iix>Вот сижу и программирую клиента для одного IM и в одной функции получился извращенный до нельзя код.
В каком смысле? В том что в нем ошибка на ошибке?
Re: Красота кода
От: Kubyshev Andrey  
Дата: 31.01.06 18:45
Оценка: -1 :)
Я бы сделал просто в таком стиле. Просто, но со вкусом:


if (!....){
  DbgPrint  ...
  goto error;
}
...
error:
if (pingthrd) {
   TerminateThread(pingthrd,0);
   CloseHandle(pingthrd);
}



Если не просто и это С++ то исключения.
Re[2]: Красота кода
От: GlebZ Россия  
Дата: 31.01.06 19:03
Оценка:
Здравствуйте, Kubyshev Andrey, Вы писали:

KA>Я бы сделал просто в таком стиле. Просто, но со вкусом:

KA>if (!....){
KA> DbgPrint ...
KA> goto error;
KA>}
KA>...
KA>error:
KA>if (pingthrd) {
KA> TerminateThread(pingthrd,0);
KA> CloseHandle(pingthrd);
KA>}
Жуть. Даже для С. Можно сделать функцию, что-то Deinintialize. И в ней по проверке handle==0 убивать. И без всяких goto. Сразу return.

KA>Если не просто и это С++ то исключения.
Неизвестно что имелось ввиду. Mutex если уже был создан, должен давать ошибку, и это есть нормальная ситуация. Он для этого и создается. А может это и исключительный случай. Без знания того что здесь автор хотел, не скажешь.
Поэтому лучше RAII.
Re[2]: Красота кода
От: iix  
Дата: 31.01.06 19:17
Оценка:
Здравствуйте, GlebZ, Вы писали:

GZ>Здравствуйте, iix, Вы писали:

iix>>Всем привет.
iix>>Вот сижу и программирую клиента для одного IM и в одной функции получился извращенный до нельзя код.
GZ>В каком смысле? В том что в нем ошибка на ошибке?
Да какойто пароноидальный код получился. Что со мной творится. Неужели я стал параноиком после прочтения книги Windows для профессионалов. Эти CloseHandel меня уже запарили. Последнии мой разработки так и кишат этими проверками. Черт кажется я потихоньку схожу с ума
Re[3]: Красота кода
От: iix  
Дата: 31.01.06 19:22
Оценка:
Здравствуйте, GlebZ, Вы писали:

GZ>Неизвестно что имелось ввиду. Mutex если уже был создан, должен давать ошибку, и это есть нормальная ситуация. Он для этого и создается. А может это и исключительный случай. Без знания того что здесь автор хотел, не скажешь.
GZ>Поэтому лучше RAII.
Просто хотел спросить как иначе но красивше без этих if внутри if написать данную функцию. И как боротся с таким количеством проверок. Чтоли принимать таблетки против паранойй
Re: Красота кода
От: iix  
Дата: 31.01.06 19:26
Оценка:
Здравствуйте, iix, Вы писали:

Может вам выслать еще одну функцию там также проверка за проверкой. Просто последнее время при программировании я слежу за выходными значениями каждой функции. Раньше было круто не парился я с этими проверками и код был компактнее а сейчач даже незнаю.............
Re[4]: Красота кода
От: GlebZ Россия  
Дата: 31.01.06 19:32
Оценка: 7 (2)
Здравствуйте, iix, Вы писали:

iix>Здравствуйте, GlebZ, Вы писали:

GZ>>Неизвестно что имелось ввиду. Mutex если уже был создан, должен давать ошибку, и это есть нормальная ситуация. Он для этого и создается. А может это и исключительный случай. Без знания того что здесь автор хотел, не скажешь.
GZ>>Поэтому лучше RAII.
iix>Просто хотел спросить как иначе но красивше без этих if внутри if написать данную функцию. И как боротся с таким количеством проверок. Чтоли принимать таблетки против паранойй 

class CMutex
{
HANDLE m_handle;
public:
CMutex()
{
  m_handle=0;
  m_handle==CreateMutex(NULL,false,NULL);
  if (!m_handle)
     throw Exception("Error");
}
~CMutex(){if (m_handle)CloseHandle(m_handle);};
}
class CThread
{
HANDLE handle;
public:
CMutex()
{
     m_handle=0;
     m_handle=(HANDLE)_beginthreadex(NULL,0,(PTHREAD)PingThread,this,0,&thrdaddr)  
     if (!m_handle)
        throw Exception("Error");
}
~CThread(){}
}

void Logging()
{
if (!Logged())
{
   CMutex sendmtx;
   CMutex recvmtx;
   CThread pingthrd;
   CThread recvthrd;
   pingthrd.TerminateThread(0);
   recvthrd.TerminateThread(0);
   mthrd[0]=pingthrd;
   mthrd[1]=recvthrd;
   WaitForMultipleObjects(2,mthrd,true,INFINITE);
}
}

По моему, вполне читабельно.
Re[5]: Красота кода
От: GlebZ Россия  
Дата: 31.01.06 19:34
Оценка:
Здравствуйте, GlebZ, Вы писали:

GZ>Здравствуйте, iix, Вы писали:

iix>>Здравствуйте, GlebZ, Вы писали:


GZ>class CThread
GZ>{
GZ>HANDLE m_handle;
GZ>public:
GZ>CMutex()
GZ>{
GZ> m_handle=0;
GZ> m_handle=(HANDLE)_beginthreadex(NULL,0,(PTHREAD)PingThread,this,0,&thrdaddr)
GZ> if (!m_handle)
GZ> throw Exception("Error");
GZ>}
GZ>~CThread(){CloseHandle(m_handle);}
void Terminate(int i){TerminateThread(m_handle, i);};
GZ>}
Сорри, конечно забыл.
Re[3]: Красота кода
От: Kubyshev Andrey  
Дата: 31.01.06 19:49
Оценка:
GZ>Жуть. Даже для С. Можно сделать функцию, что-то Deinintialize. И в ней по проверке handle==0 убивать. И без всяких goto. Сразу return.

Да не, ничего жуткого. Нормальный стиль. Можно как ты делать, а можно и как я. Внатуре !
Re[6]: Красота кода
От: iix  
Дата: 31.01.06 19:56
Оценка:
Да конечно красяво, но я не слишком люблю для простых действий делать классы обертки . Да кстати сначало ета функция выглядела не так а вот как:
bool CIMProto::Login2(char *login,char *pass,u_long status)
{
    unsigned int thrdaddr;

    if(connected && !logining)
    {
        if(sendmtx=CreateMutex(NULL,false,NULL))
        {
            DbgPrint("Login2: send CreateMutex error...");
            return false;
        }
        
        if(recvmtx=CreateMutex(NULL,false,NULL))
        {
            DbgPrint("Login2: recv CreateMutex error...");
            CloseHandle(sendmtx);
            return false;
        }
        
        if(pingthrd=(HANDLE)_beginthreadex(NULL,0,(PTHREAD)PingThread,this,0,&thrdaddr))
        {
            DbgPrint("Login2: ping _beginthreadex error...");
            CloseHandle(sendmtx);
            CloseHandle(recvmtx);
            return false;
        }

        if(recvthrd=(HANDLE)_beginthreadex(NULL,0,(PTHREAD)RecvThread,this,0,&thrdaddr))
        {
            DbgPrint("Login2: recv _beginthreadex error...");
            CloseHandle(sendmtx);
            CloseHandle(recvmtx);
            TerminateThread(pingthrd,0);
            WaitForSingleObject(pingthrd,INFINITE);
            CloseHandle(pingthrd);
            return false;
        }

        DbgPrint("Login2: successfull...")

        return true;
    }

    DbgPrint("Login2: not connected or loginning error...")

    return false;
}

И как на твой взгляд какая функция красивей?
Re[5]: Красота кода
От: Кирилл Лебедев Россия http://askofen.blogspot.com/
Дата: 31.01.06 20:05
Оценка:
Здравствуйте, GlebZ, Вы писали:

GZ>
GZ>  m_handle=0;
GZ>

Эта строка лишняя. CreateMutex и _beginthreadex сами инициализируют handle так, как надо.
С уважением,
Кирилл Лебедев
Software Design blog — http://askofen.blogspot.ru/
Re[6]: Красота кода
От: iix  
Дата: 31.01.06 20:09
Оценка:
Здравствуйте, Кирилл Лебедев, Вы писали:

КЛ>Здравствуйте, GlebZ, Вы писали:

GZ>>
GZ>>  m_handle=0;
GZ>>

КЛ>Эта строка лишняя. CreateMutex и _beginthreadex сами инициализируют handle так, как надо.
Да не в етом суть вопроса пусть хот десять раз напищет a=a+1. Здесь вопрос не в оптимизации кода а в красоте и оригинальности
Re[7]: Красота кода
От: iix  
Дата: 31.01.06 20:19
Оценка:
Здравствуйте, Я писали:

iix>И как на твой взгляд какая функция красивей?

Как вам эти многократные CloseHandle их бы как нить зделать по другому например так:

bool CIMProto::Login2(char *login,char *pass,u_long status)
{
    unsigned int thrdaddr;

    if(connected && !logining)
    {
        recvthrd=0;
        __try
        {
        if(sendmtx=CreateMutex(NULL,false,NULL))
        {
            DbgPrint("Login2: send CreateMutex error...");
            return false;
        }
        
        if(recvmtx=CreateMutex(NULL,false,NULL))
        {
            DbgPrint("Login2: recv CreateMutex error...");
            return false;
        }
        
        if(pingthrd=(HANDLE)_beginthreadex(NULL,0,(PTHREAD)PingThread,this,0,&thrdaddr))
        {
            DbgPrint("Login2: ping _beginthreadex error...");
            return false;
        }

        if(recvthrd=(HANDLE)_beginthreadex(NULL,0,(PTHREAD)RecvThread,this,0,&thrdaddr))
        {
            DbgPrint("Login2: recv _beginthreadex error...");
            return false;
        }
        }
        __finally
        {
            if(!recvthrd)
            {
            CloseHandle(sendmtx);
            CloseHandle(recvmtx);
            TerminateThread(pingthrd,0);
            WaitForSingleObject(pingthrd,INFINITE);
            CloseHandle(pingthrd);
            }
        }

        DbgPrint("Login2: successfull...");

        return true;
    }

    DbgPrint("Login2: not connected or loginning error...");

    return false;
}
Re[8]: Красота кода
От: iix  
Дата: 31.01.06 20:21
Оценка:
Или блок __finally записать в функцию и вызывать перед return false;
Re: Красота кода
От: VladD2 Российская Империя www.nemerle.org
Дата: 01.02.06 03:45
Оценка: 2 (2) +3 :))) :))) :)
Здравствуйте, iix, Вы писали:

iix>Всем привет.
iix>Вот сижу и программирую клиента для одного IM и в одной функции получился извращенный до нельзя код. Хотелось бы чтобы вы предложиле более красивую реализацию данного кода:

Во, ПК, гляди (!) как искусно "юзаются" твои любимые деструкторы и RAII. Тут хоть кол на голове теши, но пока в библиотеке не напишут грамотную обертку народ будет мучаться, плакать, но жрать этот долбанный кактус.
... << RSDN@Home 1.2.0 alpha rev. 637>>
Есть логика намерений и логика обстоятельств, последняя всегда сильнее.
Re: Красота кода
От: c-smile Канада http://terrainformatica.com
Дата: 01.02.06 07:10
Оценка:
Здравствуйте, iix, Вы писали:

Если goto не нравится то:

void foo() 
{

  if(!connected || logingin) return;

  do {
    sendmtx=CreateMutex(NULL,false,NULL); 
    if(!sendmtx) break;
    pingthrd=(HANDLE)_beginthreadex(NULL,0,(PTHREAD)PingThread,this,0,&thrdaddr);
    if(!pingthrd) break;
    ...
    DbgPrint("Login2: succesfull...");
    logingin=true;
    return true;

  } while(false);

  //ABEND handling here
}
Re[7]: Красота кода
От: iix  
Дата: 01.02.06 08:28
Оценка:
А вот к чему я пришол:

bool CIMProto::Login2(char *login,char *pass,u_long status)
{
    unsigned int thrdaddr;

    if(connected && !logining)
    {
        try
        {
            if(sendmtx=CreateMutex(NULL,false,NULL))
            {
                DbgPrint("Login2: send CreateMutex error...");
                throw 0;
            }
        
            if(recvmtx=CreateMutex(NULL,false,NULL))
            {
                DbgPrint("Login2: recv CreateMutex error...");
                throw 1;
            }
        
            if(pingthrd=(HANDLE)_beginthreadex(NULL,0,(PTHREAD)PingThread,this,0,&thrdaddr))
            {
                DbgPrint("Login2: ping _beginthreadex error...");
                throw 2;
            }

            if(recvthrd=(HANDLE)_beginthreadex(NULL,0,(PTHREAD)RecvThread,this,0,&thrdaddr))
            {
                DbgPrint("Login2: recv _beginthreadex error...");
                throw 3;
            }
        }
        catch(int errcode)
        {
            switch(errcode)
            {
            case 3:
                {
                    TerminateThread(pingthrd,0);
                    WaitForSingleObject(pingthrd,INFINITE);
                    CloseHandle(pingthrd);
                }
            case 2: CloseHandle(recvmtx);
            case 1: CloseHandle(sendmtx);
            }

            DbgPrint("Login2: error...");
            return false;
        }

        logining=true;
        DbgPrint("Login2: successfull...");
        return true;
    }

    DbgPrint("Login2: not connected or loginning error...");
    return false;
}
Re[2]: Красота кода
От: GlebZ Россия  
Дата: 01.02.06 10:33
Оценка: -2
Здравствуйте, c-smile, Вы писали:

Обзовем goto как do while и break?
А вы, батенька, и Вирта не читали?
Re[2]: Красота кода
От: Sinclair Россия https://github.com/evilguest/
Дата: 01.02.06 11:16
Оценка: +1
Здравствуйте, iix, Вы писали:

iix>Здравствуйте, iix, Вы писали:

iix>Может вам выслать еще одну функцию там также проверка за проверкой. Просто последнее время при программировании я слежу за выходными значениями каждой функции. Раньше было круто не парился я с этими проверками и код был компактнее а сейчач даже незнаю.............
Делай обертки. Делай. Это окупится. Меньше отладки будет.
1.1.4 stable rev. 510
Уйдемте отсюда, Румата! У вас слишком богатые погреба.
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.