Several methods to be handled the same way

In the chunk of C # I'm writing at the moment, I need to handle multiple methods with the same signature in the same way. There may also be more of these methods in the future. Instead of repeating the same type of logic over and over, I came up with the following:

private delegate bool cleanStep(BuildData bd, out String strFailure);

List<cleanStep> steps = new List<cleanStep>();
steps.Add(WriteReadme);
steps.Add(DeleteFiles);
steps.Add(TFSHelper.DeleteLabel);
steps.Add(TFSHelper.DeleteBuild);

List<cleanStep>.Enumerator enumerator = steps.GetEnumerator();
bool result = true;
while (result && enumerator.MoveNext())
{
   result = enumerator.Current.Invoke(build, out strFailure);
   if (!result)
   {
      logger.Write(LogTypes.Error, strFailure);
   }
}

      

I think this has some cool features, but it also feels a bit over engineering and confusing.

Can you thank for a better way to do this?

btw:

  • it doesn't have to be transactional.
  • strFailure doesn't hide exceptions, it wraps them completely when needed

Thank.

+1


source to share


4 answers


Why not use a foreach loop and just break? (I renamed cleanStep

to cleanStep

here for convention - I suggest you do the same.)

foreach(CleanStep step in steps)
{
    string failureText;
    if (!step(build, out failureText))
    {
        logger.Write(LogTypes.Error, strFailure);
        break;
    }
}

      



Note that this also conforms to a contract IEnumerator<T>

where your current code doesn't work - foreach

automatically calls Dispose

, but IEnumerator<T>

implements IDisposable

. This won't be a problem in this case, but with iterator blocks, the utility is used to execute the blocksfinally

.

+9


source


Your decision is straightforward and easy to understand. I see no reason to do it differently :)



My only suggestion is to replace your iterator with a foreach loop and break the error.

+2


source


Re obfuscated - well foreach

with a break might be clearer (plus it will be an Dispose()

enumerator which you don't).

In fact, the params cleanStep [] parameter can help:

static bool RunTargets(params cleanStep[] targets)
{
    // detail as per Jon post
}

      

then you can call:

bool foo = RunTargets(WriteReadme, DeleteFiles,
              TFSHelper.DeleteLabel, TFSHelper.DeleteBuild);

      

0


source


I would return an Exception object instead of a string. Since exceptions often have a global policy, I would write some Exception extensions. Now you get:

static Exception Run( this IEnumerable<Step> steps) {
   return 
       steps
       .FirstOrDefault( (step) => step( build ) != null )
       .LogIfFailure();  //or .ThrowIfFailure()
}

      

Extensions:

public static class ExceptionExtensions {
    private static logger = new Logger();

    public static Exception LogIfFailure( this Exception e ) {
        if( e != null )
            logger.Write( e.Message );
        return e;
    }
    public static Exception ShowDialogIfFailure( this Exception e ) {
        if( e != null )
            MessageBox.Show( e.Message );
        return e;
    }
    public static void ThrowIfFailure( this Exception e ) {
        if( e != null )
            Throw( e );
    }
}

      

0


source







All Articles