2
\$\begingroup\$

I want to clone a collection with the following method and I want to know if it can be optimized:

public T CloneCollection<T>(IEmployees employees) where T : IEmployees, new()
{
    T collection = new T();
    foreach (IEmployee employee in employees)
    {
        collection.Add(employee);
    }
    return collection;
}

These are the other classes which are related to the method above:

public class RegularEmployee : Entities, IEmployee
{
//properties
}

public class User : Entities, IEmployee
{
//properties
}

public class Manager : Entities, IEmployee
{
//properties
}

public class ManagerCollection : List<Manager>, IEmployees
{
   void Add(IEmployee employee);
}

public class RegularEmployeeCollection : List<RegularEmployee>, IEmployees
{
   void Add(IEmployee employee);
}

public class UserCollection : List<User>, IEmployees
{
   void Add(IEmployee employee);
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Can you provide the code for IEmployee, IEmployees and also the code behind Add methods from your xxxCollection types? CodeReview requires that all relevant code is provided. \$\endgroup\$ – Alexei Apr 8 '16 at 20:37
  • \$\begingroup\$ How about a HumanBeing, or some such base class - perhaps an abstract class - and implement Clone there. And perhaps the way to clone in this case is to serialize the object and then un-serialize it; which of course effectively makes a copy. \$\endgroup\$ – radarbob Apr 9 '16 at 23:19
2
\$\begingroup\$

You can replace your loop with AddRange():

public T CloneCollection<T>(IEmployees employees) where T : IEmployees, new()
{
    T collection = new T();
    collection.AddRange(employees);
}

Additional thought:

Note that your solution and this solution are simply cloning the list and not the items in the list. If you update an Employee, the employee will be modified in both lists. If this is NOT the desired behavior, you will need to clone each employee as well.

\$\endgroup\$
2
\$\begingroup\$

Review

Your code does not clone a collection. It creates one and assigns the values of the specified source collection to it. It also lacks proper generics to make this usable for other types of collections.

public T CloneCollection<T>(IEmployees employees) where T : IEmployees, new()
{
    // creates a target collection
    T collection = new T();

    // assigns items from source to target
    foreach (IEmployee employee in employees)
    {
        collection.Add(employee);
    }
    return collection;
}

Cloning a collection is ambiguously defined. It does not state whether the items should be cloned as well.

Shallow Clone

A shallow clone of a collection is a new instance, holding references to the same items as the source collection.

public static T ShallowClone<T, TItem>(T source) where T : ICollection<TItem>, new()
{
    var target = new T();
    foreach (var item in source)
    {
        target.Add(item);
    }
    return target;
}

I don't really see a use case for this. It's better using common collections or creating overloaded constructors on custom collections that take IEnumerable<T> as input. If your custom collection inherits from List<T> you should take advantage of AddRange.

Deep Clone

A deep clone of a collection is a new instance with clones of the source items. Make sure the items implement ICloneable. If you want to provide deep-cloning for items that do not implement this interface, you'd have to create an overload of DeepClone that uses a default strategy for deep-cloning items. This is elaborated in this post.

public static T DeepClone<T, TItem>(T source) 
    where T : ICollection<TItem>, new()
    where TItem : ICloneable
{
    var target = new T();
    foreach (var item in source)
    {
        target.Add(item == null ? null : item.Clone());
    }
    return target;
}

Checking input for null is left out for brevity.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.