How can I clear this ugly if statement?

Question

I have the following ugly if statement, which is part of a class that is retrieved from an IOC container:

protected virtual void ExecuteSourceControlGet(IBuildMetaData buildMetaData, IPackageTree componentTree)
{
    if ((buildMetaData.RepositoryElementList != null) && (buildMetaData.RepositoryElementList.Count > 0))
    {
        componentTree.DeleteWorkingDirectory();

        foreach (var repositoryElement in buildMetaData.RepositoryElementList)
        {
            repositoryElement.PrepareRepository(componentTree, get).Export();
        }

    }

    if((buildMetaData.ExportList != null) && (buildMetaData.ExportList.Count > 0))
    {
        var initialise = true;

        foreach (var sourceControl in buildMetaData.ExportList)
        {
            log.InfoFormat("\nHorn is fetching {0}.\n\n".ToUpper(), sourceControl.Url);

            get.From(sourceControl).ExportTo(componentTree, sourceControl.ExportPath, initialise);

            initialise = false;
        }

    }

    log.InfoFormat("\nHorn is fetching {0}.\n\n".ToUpper(), buildMetaData.SourceControl.Url);

    get.From(buildMetaData.SourceControl).ExportTo(componentTree);
}

My usual approach to exception is if the instructions are to subclass each condition.

This example is different:

• The class that has this method is retrieved from the IOC container.
• I may need the logic between 2 if statements to be executed or not to be executed at all.

Any advice is greatly appreciated.

Answer 1

How to use two extraction methods and invert if for protection:

protected virtual void ExecuteSourceControlGet(IBuildMetaData buildMetaData, IPackageTree componentTree)
{
   ExecuteRepositoryElementList(buildMetaData.RepositoryElementList, componentTree);

   ExecuteExportList(buildMetaData.ExportList, componentTree);

   log.InfoFormat("\nHorn is fetching {0}.\n\n".ToUpper(), buildMetaData.SourceControl.Url);

   get.From(buildMetaData.SourceControl).ExportTo(componentTree);
}

private void ExecuteRepositoryElementList(RepositoryElementList repositoryElements, IPackageTree componentTree)
{
   // Guard: No elements
   if (repositoryElements == null || repositoryElements.Count == 0) return;

   componentTree.DeleteWorkingDirectory();

   foreach (var repositoryElement in repositoryElements)
   {
      repositoryElement.PrepareRepository(componentTree, get).Export();
   }
}

private void ExecuteExportList(ExportList exportList, IPackageTree componentTree)
{
   // Guard: No elements
   if(exportList == null || exportList.Count == 0) return;

   var initialise = true;

   foreach (var sourceControl in exportList)
   {
      log.InfoFormat("\nHorn is fetching {0}.\n\n".ToUpper(), sourceControl.Url);

      get.From(sourceControl).ExportTo(componentTree, sourceControl.ExportPath, initialise);

      initialise = false;
   }
}

BTW: IBuildMetaData.RepositoryElementList IBuildMetaData.ExportList.

Answer 2

, if - . , :

public static bool HasElements<T>(this ICollection<T> collection)
{
    return collection != null && collection.Count != 0;
}

:

if (buildMetaData.RepositoryElementList.HasElements())

if (buildMetaData.ExportList.HasElements())

. - , . , .

, , , , , , - foreach:

public static IEnumerable<T> EmptyIfNull<T>(this IEnumerable<T> source)
{
    return source ?? Enumerable.Empty<T>();
}

( , , inline, ...)

Answer 3

protected virtual void ExecuteSourceControlGet
    (IBuildMetaData metaData, IPackageTree tree)
{
    if (metaData.RepositoryElementList.HasElements())
    {
        tree.DeleteWorkingDirectory();

        foreach (var element in metaData.RepositoryElementList)
             element.PrepareRepository(tree, get).Export();    
    }

    if(metaData.ExportList.HasElements())
    {
        var init = true;

        foreach (var sourceControl in metaData.ExportList)
        {
            log.InfoFormat
              ("\nHorn is fetching {0}.\n\n".ToUpper(), sourceControl.Url);

            get.From(sourceControl)
              .ExportTo(tree, sourceControl.ExportPath, init);

            init = false;
        }

    }

    log.InfoFormat
       ("\nHorn is fetching {0}.\n\n".ToUpper(), 
           metaData.SourceControl.Url);

    get.From(metaData.SourceControl)
       .ExportTo(tree);
}

//Credits goes to Jon Skeet...
public static class CollectionExtensions
{
    public static bool HasElements<T>(this ICollection<T> collection)
    {
        return collection != null && collection.Count != 0;
    }
}

Answer 4

I would either create an extension method, as John suggested, or either (if that makes sense in the context of the rest of your class), reorganize it and make buildMetaData a member of the class and have two private properties bool with get that mask your double sentence if the statement with something more readable.

In any case, the same result is achieved, which makes it more readable.