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);
}
dfhwze
14.1k3 gold badges40 silver badges101 bronze badges
asked Apr 8, 2016 at 19:24
\$\endgroup\$
2
  • 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\$ Commented Apr 8, 2016 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\$ Commented Apr 9, 2016 at 23:19

2 Answers 2

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.

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
answered Apr 12, 2016 at 3:33
\$\endgroup\$
0
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.

answered Jul 12, 2019 at 11:51
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.