Информация об изменениях

Сообщение Re: Покритикуйте код от 09.02.2015 3:42

Изменено 09.02.2015 3:45 omgOnoz

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

CEM>Почитал соседнюю ветку, и вспомнилось...

CEM>Принцип создания функции инициализации:

  Скрытый текст
CEM>
CEM>BOOL CApp::InitCommon()
CEM>{
CEM>    try
CEM>    {
CEM>        if(!m_zero.InitCommon())
CEM>            throw 0;
CEM>        if(!m_one.InitCommon())
CEM>            throw 1;
CEM>        if(!m_two.InitCommon())
CEM>            throw 2;
CEM>        //...
CEM>    }
CEM>    catch(int iExc)
CEM>    {
CEM>        switch(iExc)
CEM>        {
CEM>        //...
CEM>        case 3:
CEM>            m_two.UninitCommon();
CEM>        case 2:
CEM>            m_one.UninitCommon();
CEM>        case 1:
CEM>            m_zero.UninitCommon();
CEM>        }
CEM>        return FALSE;
CEM>    }
CEM>    return TRUE;
CEM>}
CEM>

CEM>у меня конкретно в коде были вызовы статических методов классов для инициализации общих данных, (через C:: ).



CEM>чтоб понятнее, в чём суть, лучше так:

  Скрытый текст
CEM>
CEM>BOOL CApp::Init()
CEM>{
CEM>    try
CEM>    {
CEM>        m_hSome0 = InitSome0()
CEM>        if (!m_hSome0)
CEM>            throw 0;
CEM>        m_hSome1 = InitSome1()
CEM>        if (!m_hSome1)
CEM>            throw 1;
CEM>        m_hSome2 = InitSome2()
CEM>        if (!m_hSome2)
CEM>            throw 2;
CEM>        //...
CEM>    }
CEM>    catch(int iExc)
CEM>    {
CEM>        switch(iExc)
CEM>        {
CEM>        //...
CEM>        case 3:
CEM>            Release(hSome2);
CEM>        case 2:
CEM>            Release(hSome1);
CEM>        case 1:
CEM>            Release(hSome0);
CEM>        }
CEM>        return FALSE;
CEM>    }
CEM>    return TRUE;
CEM>}
CEM>



CEM>Часто в примерах встречал код, который инициализировал некоторые члены класса один за другим, в случае ошибки делал деинициализацию уже готовых. В результате к хвосту функции число вызова деинициализаторов было велико. Ну и повторы кода на каждом условии.
CEM>Как вариант, люди использовали некую булеву переменную для понимания, что инициализации идут хорошо, каждая новая инициализация обкладывалась условием на эту переменную. В конце эта переменная проверялась, если false, то проверялись опять же все хендлы на валидность, и делалась деинициализация.
CEM>Мне кажется, мой код проще и нагляднее, быстро расширяется, можно не бояться забыть, что что-то пропустил.
CEM>Жду: критику, доработку, идеи

Сделать uninit / release — который ничего не делает, если ресурс не был выделен.
Если тебе так хочется JavaStyle — то пусть эксепшн кидают InitCommon / InitSome0 — а в catch вызываеться — UninitCommon() -> который вызовет m_two.UninitCommon() и т.п.

BOOL CApp::InitCommon()
{
    try
    {
        m_zero.InitCommon();
        m_one.InitCommon();
        m_two.InitCommon();
    }
    catch(ExpectedException ee)
    {
        UninitCommon();
        return FALSE;
    }
    return TRUE;
}

void CApp::UninitCommon() 
{
    m_two.UninitCommon();
    m_one.UninitCommon();
    m_zero.UninitCommon();
}
Здравствуйте, CEMb, Вы писали:

CEM>Почитал соседнюю ветку, и вспомнилось...

CEM>Принцип создания функции инициализации:

  Скрытый текст
CEM>
CEM>BOOL CApp::InitCommon()
CEM>{
CEM>    try
CEM>    {
CEM>        if(!m_zero.InitCommon())
CEM>            throw 0;
CEM>        if(!m_one.InitCommon())
CEM>            throw 1;
CEM>        if(!m_two.InitCommon())
CEM>            throw 2;
CEM>        //...
CEM>    }
CEM>    catch(int iExc)
CEM>    {
CEM>        switch(iExc)
CEM>        {
CEM>        //...
CEM>        case 3:
CEM>            m_two.UninitCommon();
CEM>        case 2:
CEM>            m_one.UninitCommon();
CEM>        case 1:
CEM>            m_zero.UninitCommon();
CEM>        }
CEM>        return FALSE;
CEM>    }
CEM>    return TRUE;
CEM>}
CEM>

CEM>у меня конкретно в коде были вызовы статических методов классов для инициализации общих данных, (через C:: ).



CEM>чтоб понятнее, в чём суть, лучше так:

  Скрытый текст
CEM>
CEM>BOOL CApp::Init()
CEM>{
CEM>    try
CEM>    {
CEM>        m_hSome0 = InitSome0()
CEM>        if (!m_hSome0)
CEM>            throw 0;
CEM>        m_hSome1 = InitSome1()
CEM>        if (!m_hSome1)
CEM>            throw 1;
CEM>        m_hSome2 = InitSome2()
CEM>        if (!m_hSome2)
CEM>            throw 2;
CEM>        //...
CEM>    }
CEM>    catch(int iExc)
CEM>    {
CEM>        switch(iExc)
CEM>        {
CEM>        //...
CEM>        case 3:
CEM>            Release(hSome2);
CEM>        case 2:
CEM>            Release(hSome1);
CEM>        case 1:
CEM>            Release(hSome0);
CEM>        }
CEM>        return FALSE;
CEM>    }
CEM>    return TRUE;
CEM>}
CEM>



CEM>Часто в примерах встречал код, который инициализировал некоторые члены класса один за другим, в случае ошибки делал деинициализацию уже готовых. В результате к хвосту функции число вызова деинициализаторов было велико. Ну и повторы кода на каждом условии.
CEM>Как вариант, люди использовали некую булеву переменную для понимания, что инициализации идут хорошо, каждая новая инициализация обкладывалась условием на эту переменную. В конце эта переменная проверялась, если false, то проверялись опять же все хендлы на валидность, и делалась деинициализация.
CEM>Мне кажется, мой код проще и нагляднее, быстро расширяется, можно не бояться забыть, что что-то пропустил.
CEM>Жду: критику, доработку, идеи

Сделать uninit / release — который ничего не делает, если ресурс не был выделен.
Если тебе так хочется JavaStyle — то пусть эксепшн кидают InitCommon / InitSome0 — а в catch вызываеться — UninitCommon() -> который вызовет m_two.UninitCommon() и т.п.

BOOL CApp::InitCommon()
{
    try
    {
        m_zero.InitCommon();
        m_one.InitCommon();
        m_two.InitCommon();
    }
    catch(ExpectedException ee)
    {
        LogError(ee);
        UninitCommon();
        return FALSE;
    }
    return TRUE;
}

void CApp::UninitCommon() 
{
    m_two.UninitCommon();
    m_one.UninitCommon();
    m_zero.UninitCommon();
}