Reputation: 10273
I am reading "Clean Code" and having trouble figuring out how to keep some of my functions (usually constructors) to their MAXIMUM of 3 parameters.
Often my objects need an awful lot of information to work - am I supposed to make a small constructor and then use mutator functions to give them all of the information? This doesn't seem any better than just using a big constructor.
As an example, I have a "MovablePatch" class. It lets the user drag a square around in a window. It needs a several parameters, including Radius, Color, Renderer, InitialPosition, and Visibility. Currently I collect all of these from my GUI and then call:
MovablePatch(int radius, Renderer* renderer, Color color, Position initial, bool visibility)
These are only some of the things that I need in this class. Can anyone suggest how else I might package this information to pass to the constructor? I don't see any obvious "break it into smaller classes" appearing here.
Upvotes: 42
Views: 8859
Reputation: 129594
You could have
MovablePatch(Renderer* renderer, CircleAppearance circleAppearance)
where CircleAppearance gathers the other info.
However, clean code and other books that generalize about what good code should look like, are aiming for 80 percent of the code out there. Your code seems to be "closer to the metal" than the typical LoB (Line of Business) variety. As such, you may run into places where certain coding ideals are not applicable.
The most important part is that you're thinking about it and trying to keep things nice and tidy! :)
Upvotes: 31
Reputation: 24174
The Named Parameter Idiom is useful here. In your case, you might have
class PatchBuilder
{
public:
PatchBuilder() { }
PatchBuilder& radius(int r) { _radius = r; return *this; }
PatchBuilder& renderer(Renderer* r) { _renderer = r; return *this; }
PatchBuilder& color(const Color& c) { _color = c; return *this; }
PatchBuilder& initial(const Position& p) { _position = p; return *this; }
PatchBuilder& visibility(bool v) { _visibility = v; return *this; }
private:
friend class MovablePatch;
int _radius;
Renderer* _renderer;
Color _color;
Position _position;
bool _visibility;
};
class MovablePatch
{
public:
MovablePatch( const PatchBuilder& b ) :
_radius( b._radius );
_renderer( b._renderer );
_color( b._color );
_position( b._position );
_visibility( b._visibility );
{
}
private:
int _radius;
Renderer* _renderer;
Color _color;
Position _position;
bool _visibility;
};
then you use it like so
int
main()
{
MovablePatch foo = PatchBuilder().
radius( 1.3 ).
renderer( asdf ).
color( asdf ).
position( asdf ).
visibility( true )
;
}
overly simplified, but I think it gets the point across. If certain parameters are required they can be included in the PatchBuilder
constructor:
class PatchBuilder
{
public:
PatchBuilder(const Foo& required) : _foo(required) { }
...
};
Obviously this pattern degenerates into the original problem if all arguments are required, in which case the named parameter idiom isn't applicable. The point being, this isn't a one size fits all solution, and as Adam describes in the comment below there are additional costs and some overhead with doing so.
Upvotes: 12
Reputation: 39903
Some of the things you are passing in could be abstracted into a larger construct. For example, visibility
, color
, and radius
, could make sense to be placed into an object that you define. Then, an instance of this class, call it ColoredCircle
, could be passed into the constructor of MovablePatch
. A ColoredCircle doesn't care where it is or what renderer it is using, but a MovablePatch does.
My main point, is that from an OO perspective, radius
isn't really an integer, it's a radius. You want to avoid these long constructor lists because it is daunting to understand the context of these things. If you collect them into a larger class, kind of like how you already have with Color
and Position
, you can have fewer parameters passed in and make it easier to understand.
Upvotes: 21
Reputation: 62005
Do not take maxims like "thou shalt not have more than 3 parameters in thy constructors" at face value. If you have the slightest chance of making an object immutable, make it; and if it being immutable means that it is going to have a constructor with 50 parameters, so be it; go for it; don't even think about it twice.
Even if the object is going to be mutable, still, you should pass its constructor as many parameters as necessary so that immediately upon construction it will be in a valid and meaningful state. In my book, it is absolutely impermissible to have to know which are the magic mutator methods that have to be called (sometimes even in the right order) before any other methods can be invoked, under penalty of segfault.
That having been said, if you would really like to reduce the number of parameters to a constructor, or to any function, simply pass this method an interface that it can invoke to get from it the stuff it needs in order to work.
Upvotes: 31
Reputation: 761
One good option is to use a Builder pattern, where each "setter" method returns the own instance, and you can chain the methods as you need.
In your case, you will get a new MovablePatchBuilder class.
The approach is very useful and you can find it in many different frameworks and languages.
Refer here to see some examples.
Upvotes: 8