views:

122

answers:

8

When I create classes, simple constructors tend to be the norm. On one of my current projects, a movie library, I have a Movie domain object. It has a number of properties, resulting in a constructor as follows:

public Movie(string title, int year, Genre genre, int length, IEnumerable<string> actors)
{
    _title = title;
    _year = year;
    _genre = genre;
    _length = length;
    _actors = new List<string>(actors);
}

This isn't terrible, but it's not simple either. Would it be worthwhile to use a factory method (static Movie CreateMovie(...)), or a perhaps an object builder? Is there any typical pattern for instantiating domain classes?

UPDATE: thanks for the responses. I was probably overthinking the matter initially, though I've learned a few things that will be useful in more complex situations. My solution now is to have the title as the only required parameter, and the rest as named/optional parameters. This seems the all round ideal way to construct this domain object.

A: 

You gave a good answer to your own question, it's the factory pattern. With the factory pattern you don't need huge constructors for encapsulation, you can set the object's members in your factory function and return that object.

DanDan
+1  A: 

This is perfectly acceptable, IMHO. I know static methods are sometimes frowned upon, but I typically drop that code into a static method that returns an instance of the class. I typically only do that for objects that are permitted to have null values.

If the values of the object can't be null, add them as parameters to the constructor so you don't get any invalid objects floating around.

Nate Bross
+2  A: 

It depends.

If that is the only constructor for that class, it means all the properties are required in order to instantiate the object. If that aligns with your business rules, great. If not, it might be a little cumbersome. If, for example, you wanted to seed your system with Movies but didn't always have the Actors, you could find yourself in a pickle.

The CreateMovie() method you mention is another option, in case you have a need to separate the internal constructor from the act of creating a Movie instance.

You have many options available to your for arranging constructors. Use the ones that allow you to design your system with no smells and lots of principles (DRY, YAGNI, SRP.)

Dave Swersky
"seed your system with Movies but didn't always have the Actors" - excellent point. I had thought about leaving out that parameter and using an `AddActor(string name)` method.
Grant Palin
I'm starting to think of having the title as the only ctor. param., and have the rest as settable properties, so I can easily add movies to the system and fill out the details later.
Grant Palin
I don't generally like to put properties in any constructors, thus keep my options open. It's very easy to handle property assignment using the new C# object initialization syntax: [new object() {fooProp = "1", barProp="Hello"}]
Dave Swersky
+3  A: 

Yes, using a factory method is a typical pattern, but the question is: Why do you need it? This is what Wikipedia says about Factory Methods:

Like other creational patterns, it deals with the problem of creating objects (products) without specifying the exact class of object that will be created. The factory method design pattern handles this problem by defining a separate method for creating the objects, which subclasses can then override to specify the derived type of product that will be created.

So, the factory method pattern would make sense if you want to return subclasses of Movie. If this isn't (and won't be) a requirement, replacing the public constructor with a factory method doesn't really serve any purpose.

For the requirements stated in your question, your solution looks really fine to me: All mandatory fields are passed as parameters to the constructor. If none of your fields are mandatory, you might want to add a default initializer and use the C# object initializer syntax.

Heinzi
Good points on the factory method. It's one creational technique I know of, so I thought I'd toss it into the fray. Given the quote, it does seem unnecessary here.
Grant Palin
+2  A: 

If you are using .NET 4.0, you can use optional/named parameters to simplify the creation of an object that accepts multiple arguments, some of which are optional. This is helpful when you want to avoid many different overloads to supply the necessary information about the object.

If you're not on .NET 4, you may want to use the Object Builder pattern to assembly your type. Object builder takes a bit of effort to implement, and keep in sync with you type - so whether there's enough value in doing so depends on your situation.

I find the builder pattern to be most effective when assembling hierarchies, rather than a type with a bunch of properties. In the latter case, I generally either overloads or optional/named parameters.

LBushkin
I am currently using 3.5, but 4.0 is an option. I also gave the builder idea a try, it is a bit of work, and seems to be overkill in this case. Though I can see it being useful in yet more complex setups, which you mentioned.
Grant Palin
A: 

I don't see anything wrong with your constructor's interface and don't see what a static method will get you. I will have the exact same parameters, right?

The parameters don't seem optional, so there isn't a way to provide an overload with fewer or use optional parameters.

From the point-of-view of the caller, it looks something like this:

 Movie m = new Movie("Inception", 2010, Genre.Drama, 150, actors);

The purpose of a factory is to provide you a customizable concrete instance of an interface, not just call the constructor for you. The idea is that the exact class is not hard-coded at the point of construction. Is this really better?

 Movie m = Movie.Create("Inception", 2010, Genre.Drama, 150, actors);

It seems pretty much the same to me. The only thing better is if Create() returned other concrete classes than Movie.

One thing to think about is how to improve this so that calling code is easy to understand. The most obvious problem to me is that it isn't obvious what the 150 means without looking at the code for Movie. There are a few ways to improve that if you wanted to:

  1. Use a type for movie length and construct that type inline new MovieLength(150)
  2. Use named parameters if you are using .NET 4.0
  3. (see @Heinzi's answer) use Object Initializers
  4. Use a fluent interface

With a fluent interface, your call would look like

 Movie m = new Movie("Inception").
   MadeIn(2010).
   InGenre(Genre.Drama).
   WithRuntimeLength(150).
   WithActors(actors);

Frankly, all of this seems like overkill for your case. Named parameters are reasonable if you are using .NET 4.0, because they aren't that much more code and would improve the code at the caller.

Lou Franco
A: 

I see nothing wrong with leaving the public constructor the way it is. Here are some of the rules I tend follow when deciding whether to go with a factory method.

  • Do use a factory method when initialization requires a complex algorithm.
  • Do use a factory method when initialization requires an IO bound operation.
  • Do use a factory method when initialization may throw an exception that cannot be guarded against at development time.
  • Do use a factory method when extra verbage may be warranted to enhance the readability.

So based on my own personal rules I would leave the constructor the way it is.

Brian Gideon
A: 

As for me - all depending on your domain model. If your domain model allows you to create simple objects - you should do it.

But often we have a lot of composite objects and the creation of each individually is too complicated. That's why we`re looking for the best way to encapsulate the logic of composite object creation. Actually, we have only two alternatives described above - "Factory Method" and "Object Builder". Creating object through the static method looks a bit strange because we placing the object creation logic into the object. Object Builder, in turn, looks to complicated.

I think that the answer lies in the unit tests. This is exactly the case when TDD would be quite useful - we make our domain model step-by-step and understand the need of domain model complexity.

madcyree