Search code examples
c#oopsingle-responsibility-principle

Is looping over an enumeration a single responsibility?


Is looping over a collection a single responsibility and do I have to implement it in its own class?

This is a concrete example for the general question: I have a class, with a method, that loops over a enumeration and deals with each item:

internal class StorageRestorer
{
    #region log4net

    private static log4net.ILog _log = log4net.LogManager.GetLogger(typeof(StorageRestorer<TEntity>));

    #endregion log4net

    private ServiceController _serviceController;

    /// <summary>
    /// Creates a new instance of the StorageRestorer class.
    /// </summary>
    /// <param name="serviceController">Service controller, where to resore the storage.</param>
    public StorageRestorer(ServiceController serviceController)
    {
        _serviceController = serviceController;
    }

    /// <summary>
    /// Restores the given storage items into the service controller.
    /// </summary>
    /// <param name="items">Storage items to be restored.</param>
    public void Restore(IEnumerable<StorageItem> items)
    {
        foreach (var item in items)
            Restore(item);
    }

    private void Restore(StorageItem item)
    {
        if (item.Status == EntityStates.Added)
        {
            _serviceController.AddObject(item.Entity);
            return;
        }

        _serviceController.AttachObject(item.Entity);
        switch (item.Status)
        {
            case EntityStates.Deleted:
                _serviceController.DeleteObject(item.Entity);
                break;
            case EntityStates.Modified:
                _serviceController.UpdateObject(item.Entity);
                break;
        }
    }
}

My question is now: does this class violate the single responsibility principle because of the loop?


Solution

  • I understand that you do not want your code to be peer reviewed, so I will stick to the questions. I do not think that your class violates the SRP. Your class is a "storage restorer", and that is just what the class does. In fact, the actual operations (add, attach, delete and upate) are implemented in another class that complies with the responsibility to carrying out the atomic operations. The code is easy to read and very understandable, so no problem there. Maybe some could argue that the code could be changed to:

    public void Restore(IEnumerable<StorageItem> items)
    {
        foreach (var item in items)
            item.Restore(_serviceController);
    }
    

    and send the responsibility of the individual item restoration to the storage item itself, but I think the class is OK as is.

    About the general question, a loop is just a mean to achieve what you want to accomplish, but not a responsibility itself. Sometimes in order to achieve what the responsibility requires, you will need to write several loops, if conditions, nested switches, and that does not mean that you will need to write a class for every structure you write. Maybe it will be a good idea to write private functions that help read the code, as long as the private functions help you achieve what the class really need to do, and nothing else (accomplish with the responsibility it has).