2
\$\begingroup\$
class Person {

    constructor(firstName, lastName, email) {
        this.firstName = firstName;
        this.lastName = lastName;
        this.email = email;
    }


    returnObject() {
       return { "firstName": this.firstName, "lastName": this.lastName, "email": this.email };
    }

    json() {
       return JSON.stringify(this.returnObject());
    }
}

Is this node.js class for returning a json well-written? This is a model class, and it's going to be used for validation and returning a json for a particular API endpoint. I am wondering if there's any improvement I can make or anything I should fix.

\$\endgroup\$
5
\$\begingroup\$

I'd get rid of the json function. Having it means you have to remember to use somePerson.json() instead of the usual JSON.stringify(somePerson). If you need to control the JSON output to make sure your API stays the same even as your class changes, you can do that using a toJSON function - JSON.stringify will respect such a function if it exists. And you have a perfectly fine toJSON function already - you just called it returnObject instead.

Though speaking of returnObject, it doesn't seem very useful in its current state. You take an object, and return an object that contain all the first object's fields. At that point, why not just return the first object itself? And once you have a function that essentially just reads return this, why have that function at all? You can't call the function without already having access to its return value anyway, right?
Granted, having it does serve to freeze the output of json (or, if you were to use this function as your toJSON, the output of JSON.stringify), which might help you avoid accidentally changing your API.

All in all, I'd probably suggest simplifying to

class Person {
    constructor(firstName, lastName, email) {
        this.firstName = firstName;
        this.lastName = lastName;
        this.email = email;
    }
}

though if you want to be sure you don't accidentally change your JSON output by changing the class (and you don't trust your test to notice if you do) you might prefer

class Person {

    constructor(firstName, lastName, email) {
        this.firstName = firstName;
        this.lastName = lastName;
        this.email = email;
    }


    toJSON() {
       return {
           "firstName": this.firstName,
           "lastName": this.lastName,
           "email": this.email
       };
    }
}
\$\endgroup\$
4
\$\begingroup\$

It is kinda useless class.

Why not just

function createPerson(firstName, lastName, email)
{
  return { firstName, lastName, email }
}

It is immediately json serializable, no need for a toJSON method. Just serialize it for the response

response.body = JSON.stringify(person)

unless a framework is already handling that for you, ie:

response.json = person
\$\endgroup\$
3
  • \$\begingroup\$ Why useless? Wouldn’t it make sense to get some typesafety? Like for instance if there is a function somewhere that should only work for objects of type Person (instead of having to validate the object in said function)..? \$\endgroup\$ Jun 28 '21 at 13:14
  • \$\begingroup\$ @dingalapadum Any validations go into the createPerson function before the return statement. We're talking about node.js aka javascript here. Even if you have a named class Person, there is no guarantee that noone modifies it after its creation. If you want typesafety, write in typescript, then the Person is just a interface and you just trust the compiler that it detects any misuse as long as your code is properly typed. \$\endgroup\$
    – slepic
    Jun 28 '21 at 18:39
  • \$\begingroup\$ Right. Sorry, I missed to mention that I was assuming typescript in the first place. \$\endgroup\$ Jun 28 '21 at 22:02
2
\$\begingroup\$

"First name" and "Last name" can be problematic in code, as not everyone has two or more names (I know at least one person with just a single-word name, for example), and even for those who do, there's little utility in knowing their writing order, other than to reassemble back into the full name.

Without some additional context, we don't know if either of those fields is a personal name, a family name, a patronymic or something else.

It's generally better to store a single "name" field.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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