loading...

How do you create entities?

razbakov profile image Aleksey Razbakov ・1 min read

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?

Discussion

markdown guide
 

I've written code like that. Say my system creates a new user with the username that they selected

addUser(name);

And it's cool. After all, a username is 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 required

addUser(name, email);
addUser(name, email, ... );

And 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

addEmployee(name=yourName, email=yourEmail, ...)

If the language doesn't support that syntax natively, just pass in like a map

addEmployee( array( "name" => $yourName, "email" => $yourEmail" ));

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.