C # / GOTO прямо здесь?

Question

О, мальчик, страсть вокруг высказываний GOTO в C #; Я боюсь даже задать этот вопрос.

Так много вопросов, подобных этому; это также заставляет меня немного нервничать. Но я серьезно.

Пожалуйста, воздержитесь от ответов, которые просто опровергают заявление GOTO оптом.

Однако я немного озадачен тем, что эта реализация не идеальна для GOTO:

public event CancelEventHandler DeleteSnapshotStarted;
public event AsyncCompletedEventHandler DeleteSnapshotCompleted;
public void DeleteSnapshot(Guid documentId, Action<Exception> callback)
{
    if (!this.Snapshots.Where(x => x.DocumentId == documentId).Any())
        throw new Exception("Snapshot not found; ensure LoadSnapshots()");

    // define action
    var _Action = new Action(() =>
    {
        // preview
        bool _Cancelled = false;
        if (DeleteSnapshotStarted != null)
        {
            CancelEventArgs _CancelArgs = new CancelEventArgs { };
            DeleteSnapshotStarted(this, _CancelArgs);
            if (_CancelArgs.Cancel)
            {
                _Cancelled = true;
                goto END;
            }
        }

        // execute
        Exception _Error = null;
        try
        {
            Proxy.CoreService.DeleteSnapshot(documentId);
            LoadSnapshots(null);
        }
        catch (Exception ex) { _Error = ex; }

    END:

        // complete
        if (DeleteSnapshotCompleted != null)
        {
            AsyncCompletedEventArgs _CompleteArgs = 
                new AsyncCompletedEventArgs(_Error, _Cancelled, null);
            DeleteSnapshotCompleted(this, _CompleteArgs);
        }

        // bubble error
        if (_Error != null)
            throw _Error;
    });

    // run it
    if (callback == null) { _Action(); }
    else
    {
        using (BackgroundWorker _Worker = new BackgroundWorker())
        {
            _Worker.DoWork += (s, arg) => { _Action(); };
            _Worker.RunWorkerCompleted += (s, arg) => { callback(arg.Error); };
            _Worker.RunWorkerAsync();
        }
    }
}

** Я даю - я избегу GOTO! : D **

Вот что кажется лучшим:

public event CancelEventHandler DeleteSnapshotStarted;
public event AsyncCompletedEventHandler DeleteSnapshotCompleted;
public void DeleteSnapshot(Guid documentId, Action<Exception> callback)
{
    if (!this.Snapshots.Where(x => x.DocumentId == documentId).Any())
        throw new Exception("Snapshot not found; ensure LoadSnapshots()");

    // define action
    var _Action = new Action(() =>
    {
        // preview
        CancelEventArgs _CancelArgs = new CancelEventArgs { };
        if (DeleteSnapshotStarted != null)
            DeleteSnapshotStarted(this, _CancelArgs);

        // execute
        Exception _Error = null;
        if (!_CancelArgs.Cancel) try
            {
                Proxy.CoreService.DeleteSnapshot(documentId);
                LoadSnapshots(null);
            }
            catch (Exception ex) { _Error = ex; }

        // complete
        if (DeleteSnapshotCompleted != null)
            DeleteSnapshotCompleted(this, 
              new AsyncCompletedEventArgs(null, _CancelArgs.Cancel, null));

        // bubble
        if (_Error != null)
            throw _Error;
    });

    // run it
    if (callback != null)
    {
        using (BackgroundWorker _Worker = new BackgroundWorker())
        {
            _Worker.DoWork += (s, arg) => { _Action(); };
            _Worker.RunWorkerCompleted += (s, arg) => 
                            { callback(arg.Error); };
            _Worker.RunWorkerAsync();
        }
    }
    else
        _Action();
}

Спасибо всем.

Answer 1

Да, у вас даже есть переменная флага:

if (!this.Snapshots.Where(x => x.DocumentId == documentId).Any())
        throw new Exception("Snapshot not found; ensure LoadSnapshots()");

    // define action
    var _Action = new Action(() =>
    {
        // preview
        bool _Cancelled = false;
        if (DeleteSnapshotStarted != null)
        {
            CancelEventArgs _CancelArgs = new CancelEventArgs { };
            DeleteSnapshotStarted(this, _CancelArgs);
            if (_CancelArgs.Cancel)
            {
                _Cancelled = true;
                <s>goto END;</s>
            }
        }

        <b>if(!_Cancelled) {</b>
            // execute
            Exception _Error = null;
            try
            {
                Proxy.CoreService.DeleteSnapshot(documentId);
                LoadSnapshots(null);
            }
            catch (Exception ex) { _Error = ex; }
        <b>}</b>
    <s>END:</s>

        // complete
        if (DeleteSnapshotCompleted != null)
        {
            AsyncCompletedEventArgs _CompleteArgs = 
                new AsyncCompletedEventArgs(_Error, _Cancelled, null);
            DeleteSnapshotCompleted(this, _CompleteArgs);
        }

        // bubble error
        if (_Error != null)
            throw _Error;
    });

Answer 2

Измените

        if (_CancelArgs.Cancel)
        {
            _Cancelled = true;
            goto END;
        }

на это:

_Cancelled = _CancelArgs.Cancel;

и КОНЕЦ: на это:

if(!Cancelled)
{
   // complete...

Answer 3

В принципе, для удобства чтения стоит избегать нелокальных ветвлений в коде.В вашем случае можно реструктурировать поток управления с помощью переменной flag.Подробности см. В ответах @NeilN и @minitech.

На практике иногда (в редких случаях :) полезно использовать goto для разрешения сложных сценариев управления, где обычные if/else/break/while/for структурыбольше вложенных или запутанных, чем необходимо.

Лучшее «хорошее» использование goto (о котором я могу думать прямо сейчас) - это вырваться из глубоко вложенного набора циклов без дополнительных дополнительных условных проверок накаждая итерация циклаС одного уровня вложенности вы можете использовать break - но со многими вложенными уровнями это становится более болезненным.Вот пример:

// Column-ordered, first value search:
int valueFound = 0;
for (int i = 0; i < x; i++)
{
    for (int j = 0; j < y; j++)
    {
        if (array[j, i] < targetValue)
        {
            valueFound = array[j, i];
            goto Found;
        }
    }
}

Console.WriteLine("No value was found.");
return;

Found:
    Console.WriteLine("The number found was {0}.", valueFound);

Answer 4

Почему бы не просто if (!_Cancelled) вокруг этих строк между GOTO и этикеткой?

Answer 5

Судя по всему, вы можете обернуть try/catch в if (!_Cancelled) { ... }. В настоящее время, как у вас есть (из кода, который вы сделали доступным), вы нигде не используете _Cancelled Новый код будет выглядеть так:

public event CancelEventHandler DeleteSnapshotStarted;
public event AsyncCompletedEventHandler DeleteSnapshotCompleted;
public void DeleteSnapshot(Guid documentId, Action<Exception> callback)
{
    if (!this.Snapshots.Where(x => x.DocumentId == documentId).Any())
        throw new Exception("Snapshot not found; ensure LoadSnapshots()");

    // define action
    var _Action = new Action(() =>
    {
        // preview
        bool _Cancelled = false;
        if (DeleteSnapshotStarted != null)
        {
            CancelEventArgs _CancelArgs = new CancelEventArgs { };
            DeleteSnapshotStarted(this, _CancelArgs);
            if (_CancelArgs.Cancel)
            {
                _Cancelled = true;
            }
        }

        if (!_Cancelled) {
            // execute
            Exception _Error = null;
            try
            {
                Proxy.CoreService.DeleteSnapshot(documentId);
                LoadSnapshots(null);
            }
            catch (Exception ex) { _Error = ex; }
        }

        // complete
        if (DeleteSnapshotCompleted != null)
        {
            AsyncCompletedEventArgs _CompleteArgs = 
                new AsyncCompletedEventArgs(_Error, _Cancelled, null);
            DeleteSnapshotCompleted(this, _CompleteArgs);
        }

        // bubble error
        if (_Error != null)
            throw _Error;
    });

    // run it
    if (callback == null) { _Action(); }
    else
    {
        using (BackgroundWorker _Worker = new BackgroundWorker())
        {
            _Worker.DoWork += (s, arg) => { _Action(); };
            _Worker.RunWorkerCompleted += (s, arg) => { callback(arg.Error); };
            _Worker.RunWorkerAsync();
        }
    }
}

Answer 6

Попробуйте обернуть удаление снимка этим и удалите метку GOTO и END

if(!_Cancelled)
{
    Exception _Error = null;
        try
        {
            Proxy.CoreService.DeleteSnapshot(documentId);
            LoadSnapshots(null);
        }
        catch (Exception ex) { _Error = ex; }
}

Answer 7

В общем: Вместо этого было бы гораздо яснее и удобнее обслуживать код, чтобы он не подвергался рефакторингу. Это большой метод, и он должен быть немного разбит.

Иногда goto - хороший выбор, но большую часть времени его используют, когда достаточно простого рефакторинга.

В вашем случае: В вашем случае, как видно из множества других ответов, предлагающих использование отмененных флагов, можно решить вашу проблему без перехода.

Answer 8

Потому что выключатель ... перерыв мог бы сделать работу чище.

Сначала вы объявляете CancleArgs как enum {... Cancelled, ..., END}

switch (CancleArgs)
 {
    case Canceled: _Cancelled = true; break;
 ... other stuff
    case END:
      { 
      // complete          
      if (DeleteSnapshotCompleted != null)          
       {              
         AsyncCompletedEventArgs _CompleteArgs = new AsyncCompletedEventArgs(_Error, _Cancelled, null);              
         DeleteSnapshotCompleted(this, _CompleteArgs);
       }            
      // bubble error          
      if (_Error != null) throw _Error;  
      }
      break;
 }
// run it
   ...

Никакого рефакторинга, потому что он должен был быть написан таким образом для начала. ;)

Answer 9

public event CancelEventHandler DeleteSnapshotStarted;
public event AsyncCompletedEventHandler DeleteSnapshotCompleted;
public void DeleteSnapshot(Guid documentId, Action<Exception> callback)
{
    if (!this.Snapshots.Where(x => x.DocumentId == documentId).Any())
        throw new Exception("Snapshot not found; ensure LoadSnapshots()");

    // define action
    var _Action = new Action(() =>
    {
        // preview
        bool _Cancelled = false;
        if (DeleteSnapshotStarted != null)
        {
            CancelEventArgs _CancelArgs = new CancelEventArgs { };
            DeleteSnapshotStarted(this, _CancelArgs);
            if (_CancelArgs.Cancel)
            {
                _Cancelled = true;
            }
        }

        if(!_Cancelled)
        {
            // execute
            Exception _Error = null;
            try
            {
                Proxy.CoreService.DeleteSnapshot(documentId);
                LoadSnapshots(null);
            }
            catch (Exception ex) { _Error = ex; }
        }

        // END:

        // complete
        if (DeleteSnapshotCompleted != null)
        {
            AsyncCompletedEventArgs _CompleteArgs = 
                new AsyncCompletedEventArgs(_Error, _Cancelled, null);
            DeleteSnapshotCompleted(this, _CompleteArgs);
        }

        // bubble error
        if (_Error != null)
            throw _Error;
    });

    // run it
    if (callback == null) { _Action(); }
    else
    {
        using (BackgroundWorker _Worker = new BackgroundWorker())
        {
            _Worker.DoWork += (s, arg) => { _Action(); };
            _Worker.RunWorkerCompleted += (s, arg) => { callback(arg.Error); };
            _Worker.RunWorkerAsync();
        }
    }
}

Answer 10

В вашем коде нет необходимости использовать goto.Это только усложняет.Вот эквивалентная версия без него.

public event CancelEventHandler DeleteSnapshotStarted;
public event AsyncCompletedEventHandler DeleteSnapshotCompleted;
public void DeleteSnapshot(Guid documentId, Action<Exception> callback)
{
    if (!this.Snapshots.Where(x => x.DocumentId == documentId).Any())
        throw new Exception("Snapshot not found; ensure LoadSnapshots()");

    // define action
    var _Action = new Action(() =>
    {
        // preview
        bool _Cancelled = false;
        if (DeleteSnapshotStarted != null)
        {
            CancelEventArgs _CancelArgs = new CancelEventArgs { };
            DeleteSnapshotStarted(this, _CancelArgs);
            if (_CancelArgs.Cancel)
            {
                _Cancelled = true;
            }
        }

        if (!_Cancelled) {
          // execute
          Exception _Error = null;
          try
          {
              Proxy.CoreService.DeleteSnapshot(documentId);
              LoadSnapshots(null);
          }
          catch (Exception ex) { _Error = ex; }
        }  

        // complete
        if (DeleteSnapshotCompleted != null)
        {
            AsyncCompletedEventArgs _CompleteArgs = 
                new AsyncCompletedEventArgs(_Error, _Cancelled, null);
            DeleteSnapshotCompleted(this, _CompleteArgs);
        }

        // bubble error
        if (_Error != null)
            throw _Error;
    });

    // run it
    if (callback == null) { _Action(); }
    else
    {
        using (BackgroundWorker _Worker = new BackgroundWorker())
        {
            _Worker.DoWork += (s, arg) => { _Action(); };
            _Worker.RunWorkerCompleted += (s, arg) => { callback(arg.Error); };
            _Worker.RunWorkerAsync();
        }
    }
}