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.
3 Answers 3
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
};
}
}
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
-
\$\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\$dingalapadum– dingalapadum2021年06月28日 13:14:31 +00:00Commented Jun 28, 2021 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– slepic2021年06月28日 18:39:58 +00:00Commented Jun 28, 2021 at 18:39
-
\$\begingroup\$ Right. Sorry, I missed to mention that I was assuming typescript in the first place. \$\endgroup\$dingalapadum– dingalapadum2021年06月28日 22:02:08 +00:00Commented Jun 28, 2021 at 22:02
"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.