I recently discovered a code like this:
function createLead(
$salutation,
$firstName,
$lastName,
$email
) {
$lead = new Lead();
$lead->setSalutation($salutation);
$lead->setFirstName($firstName);
$lead->setLastName($lastName);
$lead->setEmail($email);
}
It is painful for me to work with code like this, because adding a new field to an entity will require not only changes in entity, but everywhere where creation is triggered, and the worst part is that it's super easy to misplace the argument and have unexpected results.
Do you see any pattern/anti-pattern in this code? Would you consider that to be a good or bad practice? Would you refactor it?
Top comments (3)
I've written code like that. Say my system creates a new user with the username that they selected
And it's cool. After all, a
usernameis a required field so it makes sense that an interface for creating a user should take the name. But then we decided oh hey we want some other fields to be requiredAnd everytime our spec changes we just tack on a new method.
If your spec doesn't change, I think it's fine, but of course if it does change, code gets messy.
When I was coding in python, I came across keyword-arguments
If the language doesn't support that syntax natively, just pass in like a map
Kind of thing. Of course it doesn't solve the problem if you have NEW required fields because I still need to go out there and change all calls to
addEmployee, but at least by taking an object of values I don't need to worry about changing method signatures which breaks every code out there if I wanted to add a new field.But I think if you're changing a method signature, it's probably a big enough change that all the callers need to be changed as well.
I think if there's ORM in place then better just use entities, form and validators that will do their work just fine.
Not sure why the constructor isn't used for setting the values.
also what if the user should not see the email, does it get striped or is it optional. would the lead break other code if email was not set. or should the lead get the email set to null before send to clients. In OOP there are always all these questions and in different project with different teams and people, you get to other conclusions.