Лучшая практика использования goto - PullRequest
6 голосов
/ 30 марта 2011

Правильно ли использовать goto в этом коде?Есть ли альтернативы?

return ExecuteReader(cmd, reader =>
{
    List<BEPartnership> partnerhip = null;

    //Partnership
    if (!((SqlDataReader) reader).HasRows)
        goto exit;

    partnerhip = 
        new List<BEPartnership>{new BEPartnership().GetFromReader(reader)};

    //Customers
    if (!reader.NextResult() && !((SqlDataReader) reader).HasRows)
        goto exit;

    foreach (BEPartnership p in partnerhip)
        p.Partner = new BECustomer().GetFromReader(reader);

    //Contracts
    if (!reader.NextResult() && !((SqlDataReader) reader).HasRows)
        goto exit;

    List<BEContractB2B> contracts = new List<BEContractB2B>();
    contracts.Add(new BEContractB2B().GetFromReader(reader));
    // contracts = new BEContractB2B().GetFromReader2(reader).ToList();

    exit:
    return partnerhip;
});

Ответы [ 5 ]

18 голосов
/ 30 марта 2011

Вы можете заменить каждое goto exit; на return null; или return partnerhip;, если хотите вернуть текущий заполненный список. (Полагаю, партнерство - это крутой партнер?)

7 голосов
/ 30 марта 2011

Я бы сказал нет.

Я программирую на C # с 2001 года и никогда не использовал goto!

Если вам нужен выход "короткого замыкания" в вашем коде, почему бы и нетзамените

goto exit:

на

return partnership
5 голосов
/ 30 марта 2011

goto и "лучшая практика", на мой взгляд, являются взаимоисключающими (и, вероятно, / возможно, в большинстве других).Необходимость goto указывает на неправильную конструкцию кода.В вашем случае решение кажется простым: я думаю, вам просто нужно заменить goto exit на return partnerhip и удалить метку exit:.(Следует ли читать «партнерство» вместо «партнерство»?)

1 голос
/ 30 марта 2011

В конце концов вы загружаете контракты с ридера, если он там есть. Будет намного лучше, если вы сделаете это намерение явным с помощью простого оператора if.

Измените конец на:

if (reader.NextResult() || ((SqlDataReader) reader).HasRows)
{
    List<BEContractB2B> contracts = new List<BEContractB2B>();
    contracts.Add(new BEContractB2B().GetFromReader(reader));
}

return partnerhip;

Хотя кажется, что вы просто игнорируете список контрактов ... он ничего не делает. Если создание нового класса BEContractB2B не имеет каких-либо глобальных побочных эффектов (плохие новости), вы можете просто полностью избавиться от него ..

Измените первый переход на

if (!((SqlDataReader) reader).HasRows)
    return null;

Поскольку это то, что вы делаете, вы должны дать понять, что вы вернете ноль.

0 голосов
/ 30 марта 2011

Я нашел следующий случай, когда goto может быть немного полезным: Когда использовать Goto при программировании на C .Но « Никогда не используйте GOTO » было первым, что я узнал в университете, и поэтому я действительно никогда не использовал его (по крайней мере, в C, C ++, C #, Java, ...).

Самая большая проблема GOTO заключается в том, что если вы читаете часть метода, вы не видите откуда он может быть вызван.Например:

int a = 1;
division:
int b = 4 / a;

... звучит нормально.Но вы написали сбой с делением на 0, если после блока деления есть следующий GOTO:

int a = 1;
division:
int b = 4 / a;
// ... hundreds of lines ...
a = 0;
goto division;

... Или сбой с нулевым исключением, если перед блоком разделения стоит GOTO:

goto division;
// ... hundreds of lines ...
int a = 1;
division:
int b = 4 / a;

... это только один пример, GOTO приводит к гораздо более спорным ситуациям.Так что , пожалуйста, забудьте о GOTO, и люди (включая вас) будут счастливее, читая ваш код.

Используйте "return partnership";вместо твоего гото.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...