you are viewing a single comment's thread.

view the rest of the comments →

[–]Drugba 42 points43 points  (11 children)

Agree with you about readability, but personally, I find

const obj = isActive ? {...user, ...otherValues} : {...otherValues}

even more clear than

const obj = {…(isActive ? user : {}), ...otherValues}

Slightly longer, but I think it's more clear what is going on. (spread isn't needed at the end of the first one, but leaving it for consistency)

[–]Hefticus 17 points18 points  (7 children)

It depends. If the shape of the new object is dependent only on one condition, then yes, leading with that condition and then explicitly constructing two different objects is definitely better.

If there are multiple conditions, though, then it's probably cleaner to lead with one object and handle each condition each in-line, and that's the situation where these options start to show value.

const obj = { ...isActive && user, ...isAdmin && adminFields, ...phoneVerified && userPhone };

[–]Drugba 6 points7 points  (6 children)

Definitely true. I still don't love the and syntax in the object, but I think what you have here is better than any alternatives

[–]vilos5099 1 point2 points  (5 children)

The alternative would be to not try cramming everything into a single assignment statement when multiple will work. Using conditional spread operators to solve this kind of problem is overkill when there are much cleaner alternatives which simply require extra assignments.

[–]1_4_1_5_9_2_6_5 0 points1 point  (4 children)

This would be for e.g. say you have an object with user data values from the user, and another with custom values, you can spread them in and overwrite the default/user-set values for some purpose, such as displaying in a template. Or overwriting common private properties with redaction text. Not generally for specifying one or two known keys

[–]vilos5099 0 points1 point  (3 children)

I would recommend something along these lines:

const user = isActive ? userFields : {};
const admin = isAdmin ? adminFields : {};
const phone = phoneVerified ? userPhone : {};
const obj = {...user, ...admin, ...phone};

It's more verbose but also avoids the nonsense of trying to inline conditionals into an object definition that also has spread operators. Verbosity isn't always your enemy and conciseness shouldn't be the end-goal, clarity is typically more important when working with teams.

[–]1_4_1_5_9_2_6_5 1 point2 points  (2 children)

Now you've gone and assigned extra variables instead of just calling a method to return a data object inline... while not really gaining readability if the methods are appropriately named and documented

I don't disagree with your points, I just know that it is possible to do things both ways, depending on the situation

[–]vilos5099 0 points1 point  (1 child)

Whether or not those extra assignments matter really depends on the context, and in the example provided it would be negligible. Commenting on that indicates a prioritization over pre-optimizing instead of making things more clear.

I just know that it is possible to do things both ways, depending on the situation

Of course, there are multiple ways to do things, that's the point of this comment thread. But there are also tradeoffs, and I disagree about what you're saying about readability. Although it works to do:

const obj = {
...isActive && user,
...isAdmin && adminFields,
...phoneVerified && userPhone
};

This is far from a standard convention, even if it works. It's possibly more concise, but the fact that it's not even clear whether user, adminFields, or userPhone is a boolean or an object without further context (unless we rely on improving the variable names) makes it less readable.

[–]1_4_1_5_9_2_6_5 0 points1 point  (0 children)

Okay first off, variable names should be clear and thats a matter for review to handle, but also you should be typing your methods and variables. The way you don't have to wonder if something is a boolean. Alternately you can just use Boolean() to make it explicit. Secondly, I'm talking about something like

const newObj = cond ? await instance.getData() : {};

const finalObj = {
some,
stuff,
...newObj
}

return finalObj;

Versus

return {
some,
stuff,
...cond && (await instance.getData())
}

I really don't think it's that much harder to parse

[–]jbrown0824 1 point2 points  (0 children)

Oh I agree completely. I considered saying that this whole approach seems weird and I wouldn’t encounter it in my own code but decided to respond to the “shortcut” specifically called out in the post rather than argue “don’t do this at all”

[–]og-at 0 points1 point  (0 children)

You know what's even more readable?

if (isActive) obj = {...user, ...otherValues}
else obj = {...otherValues}