3

Currently, I have a model class that represents a user.
This class has a constructor that takes an object with all user properties, used, for example, when creating the user. In this case I instantiate the model passing it the necessary information, so its constructor can do the job, then it will use its own createUser method. Fine.

But in some cases, I don't need to pass all this information to the constructor.
For example, the model has an UdpateCustomUserName method that just need userId in order to instantiate from the database and perform this method and update a single field from a single already existing record in the database.

The way I dealt with this is to make the properties information needed by the constructor optional, so when I need to call UdpateCustomUserName I just pass to the model the ID and I can instantiate and reach this method.

Is this the proper way of doing it?
Or is it a bad practice to have optional constructor parameters? In this case I would extract UdpateCustomUserName and keep it in the same module but as a standalone.

4 Answers 4

4

Optional parameters is not a bad practice, definitely not, but sometimes the optional parameters may be optional solely because of what the class does.

You arrived at a problem which is a result of a bad design. You are trying to figure out how to ignore certain properties of a model if you don't use them. The problem is, you are mixing responsibilities of classes together and trying to create a class which knows and does many things. That is pretty bad and might cause you troubles later on, because it will be very difficult to reuse the class in a different environment, for example.

Obviously, when you have a class which can do many operations, there will be a situation where one operation will not need everything that the class contains, but is required for other operations.

What you should be after is splitting your current class into several smaller classes, each being responsible for only one thing and make the class do the thing well. You could have a UserModel, which could be an actual representation of a user in your code, you could then have some sort of service, which would accept the user and do an operation on it. Obviously, if the operation required database access, you would need to provide a class acting as the database accessor to the service class as a dependency as well.

I personally would opt for refactoring the monolithic class into several smaller ones, each taking only the dependency (or dependencies) which it really needs.

2
  • 2
    Nice answer, because it goes to the bigger picture in which the OP finds themself, and to the appropriateness of the domain abstraction the OP is trying to provide.
    – Erik Eidt
    Commented Jun 15, 2016 at 16:38
  • Thanks for the great answer, I will definitely refactor. I really felt my code was bad. Some of the task that need to be performed, such as add data into a specific field, might as well be in a function, not necessarily in a class. People always mention class, to the point it seems 'weird' to just have floating function in module files... which in my case would have been the natural way to do things if I had not forced it into the class.
    – Benj
    Commented Jun 15, 2016 at 16:55
2

Having optional parameter constructors eliminates the need for multiple constructor overloads.

In Java, there's an entire software pattern for this called the Builder Pattern, which essentially replaces multiple constructors with a fluent interface. It's an elaborate, overly complex and ultimately unsatisfying pattern; optional constructor parameters is a far better solution.

1
  • It depends on how much arguments your class can have. A UserProfile can become quite big with a lot of optional fields. In those cases builder pattern works better for future compatibility. It also works nicely with collections and varargs.
    – Pepster
    Commented Jun 16, 2016 at 22:18
0

Yes and No. It Depends.

Scenario 1: You are the sole developer on the project or its your personal project.

Practices do not matter. What matters is that it works and its of good quality.

Scenario 2: There are muliple people working on this project

Yes it is a bad practice. Only required fields must be part of constructor parameters. Either use overloaded constructors or break down the class into multiple ones. Also remember, boring code that the team understands is better than clever code that only one person understands.

0

Active record design pattern, that according to wikipedia...

...is an approach to accessing data in a database. A database table or view is wrapped into a class. Thus, an object instance is tied to a single row in the table. After creation of an object, a new row is added to the table upon save. Any object loaded gets its information from the database. When an object is updated, the corresponding row in the table is also updated. The wrapper class implements accessor methods or properties for each column in the table or view.

..., seems to be the closest formal description of...

In this case I instantiate the model passing it the necessary information, so its constructor can do the job, then it will use its own createUser method.

...that dismisses the arguments for changing the implementation that otherwise could be changed just when there are changes in the use cases to support that a different formal approach is a plainer solution for.

About...

Optional data in the constructor, bad practice?

...the answer it is subjective: (1) there is a class with a single constructor instantiating common instances or specialised instances, the narrow down done by passing an additional parameter that is one too many responsibilities for a constructor since it instantiate the class and then decides to specialise the instance or (2) there is a class with a single constructor instantiating it with every thing needed to support the use case requesting the instance.

1
  • 1
    > that dismisses the arguments for changing the implementation that otherwise could be changed just when there are changes in the use cases to support that a different formal approach is a plainer solution for. Could you rephrase? I fail to parse the grammar.
    – Basilevs
    Commented Sep 7 at 16:15

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