Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

We have a Transaction class that's very loaded; so loaded that I originally ended up passing almost 20 argument to the ctor. After extracting a few value objects, there are still 12 arguments left, which I still think is too much.

How would I go at avoiding this? I think it's reasonable the arguments are passed to the constructor since they're all required, and I want to make that explicit. I also like how if I add a property, I can add it to the ctor and let my compiler find the places it broke, instead of having to rely on tests for this per se. I don't think object initializers, or builders do the problem any good. It might become more obvious in the next coming days which arguments belong together, and could be composed.

public class MyEntity() 
{
    public MyEntity(ValueType prop2, ValueType prop3, ...) 
    {
        Id = Guid.NewGuid();
        Prop2 = prop2;
        Prop3 = prop3;
        ...
    }

    public Guid Id { get; private set; }

    public ValueType Prop2 { get; private set; }

    public ValueType Prop3 { get; private set; }

    public ...
}
share|improve this question
6  
If they are required, then they are required. However you may find that some parameters are only used in some methods in which case you could extract those out into another class. Without more code though it's too hard to help with this question. Currently as it stands it might be a better fit for Programmers SE? –  dreza Jan 15 '13 at 20:11
1  
Are there natural groupings of parameters? Do you have, ex, parameters for CustomerName, CustomerAddress & etc? Combine those into a Customer POCO. If you've already done this ("After extracting a few value objects"), maybe your ctor could benefit from default values, or you might also split out your constructors to several different signatures. –  Reacher Gilt Jan 15 '13 at 20:20
add comment

closed as off topic by svick, Corbin, palacsint, dreza, Brian Reichle Jan 16 '13 at 8:11

Questions on Code Review Stack Exchange are expected to relate to code review request within the scope defined by the community. Consider editing the question or leaving comments for improvement if you believe the question can be reworded to fit within the scope. Read more about reopening questions here.If this question can be reworded to fit the rules in the help center, please edit the question.

2 Answers

It's hard to help without knowing more about your code and why do you have so many parameters.

Here are some possible reasons for many parameters and how would you fix that:

  1. Your class is doing too much, it violates the single responsibility principle. You should split it into multiple classes, each with smaller number of arguments.
  2. You're using some of the parameters only to construct some other object. You should have that object directly as a parameter. Dependency injection could help you with this.
  3. The parameter types are badly designed, maybe some of them should be combined into a single type.
share|improve this answer
add comment

I would suggest you use the Builder Pattern for this. This allows you to first instanciate a builder which is essentially a container for all your constructor arguments. The builder would be an inner class of MyEntity and would have setter methods for the different parameters. This has the advantage that you get descriptive names for passing your parameters. Also, should some parameters have sensible defaults, you can still make them optional.

MyEntity entityBuilder = MyEntity.newBuilder()
    .setProp1(someProp1)
    .setProp2(someProp2)
    .build()

The build() method could throw some exception if the combination of parameters is nonsensical, for instance if there is a missing parameter.

share|improve this answer
3  
This has the advantage that you get descriptive names for passing your parameters. -- this is already in C#, though. –  Reacher Gilt Jan 15 '13 at 20:26
add comment

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