Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I wish to project a collection from one type (Something) to another type (SomethingElse). Yes, this is a very open-eneded question, but which of the two options below do you prefer?

Creating a new instance using new:

var result = query.Select(something => new SomethingElse(something));

Using a factory:

var result = query.Select(something => SomethingElse.FromSomething(something));

When I think of a projection, I generally think of it as a conversion. Using new gives me this idea that I'm creating new objects during a conversion, which doesn't feel right. Semantically, SomethingElse.FromSomething() most definitely fits better. Although, the second option does require addition code to setup a factory, which could become unnecessarily compulsive.

share|improve this question
    
I'm pretty sure you can shorten down the second option using the factory method itself as action: var result = query.Select(SomethingElse.FromSomething); –  Spoike Nov 9 '12 at 13:51
    
@Spoike: You are correct, but my example was just trivial :) –  davenewza Nov 9 '12 at 14:00

5 Answers 5

up vote 1 down vote accepted

This question appears to be more about the appropriateness of using a factory method over a straight constructor call. My favorite example uses the TimeSpan type because it has several constructor overloads and several FromXXX static factory methods.

// how easily can you spot the bug?
int[] durationMs = { 1, 2, 3, 4 };

IEnumerable<TimeSpan> durations1 = 
    durationMs.Select(ms => new TimeSpan(0, 0, ms));

IEnumerable<TimeSpan> durations2 =
    durationMs.Select(ms => TimeSpan.FromSeconds(ms));

Which expression was the bug easier to spot in the two equivalent projections? Fixing the two expressions is also interesting:

// spot the bug after the fix?
IEnumerable<TimeSpan> durations1 = 
    durationMs.Select(ms => new TimeSpan(0, 0, 0, ms));

IEnumerable<TimeSpan> durations2 =
    durationMs.Select(ms => TimeSpan.FromMilliseconds(ms));

The durations2 expression is correct (bug fixed). However, adding just one 0 parameter to the durations1 constructor call was not enough - we have to add another 0 - going from the 3 parameter version to the 5 parameter version in order to specify milliseconds (as documented on msdn).

The "Framework Design Guidelines" book has some great advice on when to provide static factory methods. The book goes into more detail on each point whereas the link I provided merely summarizes the advice. Hope this helps.

share|improve this answer
    
Excellent answer with a perfect example - I will use this in future :) –  davenewza Dec 9 '12 at 8:42

In .NET Framework, the most frequently used convention is:

something.ToSomethingElse();

Examples:

  • 37.ToString();

  • new [] { "Hello", "World" }.ToArray();

  • DateTime.Now.ToBinary();

  • Guid.Empty.ToByteArray();

  • etc.

Note that depending on the context, maybe you could simply use Enumerable.Cast:

var result = query.Cast<SomethingElse>();

If you have a good reason to avoid this convention (for example if SomethingElse is a string), then use whatever convention you prefer from the ones you quoted in your question. They are both valid.


Another nice thing in C# is the implicit operator. For example, instead of:

var element = new XElement(new XName("section"), ...);

you simply write:

var element = new XElement("section"); // Uses implicit conversion from string to XName.
share|improve this answer
2  
something.ToSomethingElse() would mean that the responsibility and logic of creating a new SomethingElse would reside in Something. Surely this is not correct? My reasoning: A) We might not always be able to extend Something, B) Constructing new instances (factories/constructors) should naturally lie within the class in question, and C) What if we can create a SomethingElse from many different other types? Then we would have factory logic spread all over instead of in one place. –  davenewza Nov 9 '12 at 11:40
    
@davenewza: this is still the convention used by .NET Framework itself. As I said, you're not forced to use it and use one of your conventions. –  MainMa Nov 9 '12 at 11:42
1  
Cast() most likely won't work, it doesn't call cast operators. –  svick Nov 9 '12 at 12:05
1  
@davenewza agree 100% with everything you said in your comment. I always favour the factory method approach –  MattDavey Nov 9 '12 at 12:11
4  
Dave, I agree with you in part, but you can also implement the .ToSomethingElse as an extension method which would avoid polluting the original class. –  Ian Nov 9 '12 at 12:50

The problem with the second approach is that it requires a named (as opposed to an anonymous) type; otherwise, it would be impossible to pass something to a factory method. A large percentage of non-trivial LINQ queries use anonymous types, for example

query.Zip(otherQuery, (a, b) => new {First = a, Second = b}).Select(...)

or

query.Select((v,i)=> new {Value=v, Index=i}).Where(...).Select(...)

and so on. Having to define a named type for the anonymous type or using a generic Tuple<...> would solve the problem at the expense of readability.

Using new, on the other hand, lets you stay consistent even when you enter the territory of non-trivial LINQ queries. I would definitely recommend staying with new, and avoid making the matters that are already complex even more complex for no good reason.

share|improve this answer

If you want to change one type to another type but dont want to above code every time .. use automapper .It does these translation and more code reuse can be possible through it.

http://www.automapper.org/

let me know if you need highlight on same.

share|improve this answer

Personally I would either go with the first option if it's only used in one place, or create an extension method if it is used in multiple places.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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