mediamonks / frontend-coding-standards

Media.Monks - Frontend Coding Standards
60 stars 23 forks source link

First setup of README.md #1

Closed ThijsTyZ closed 3 years ago

selewi commented 4 years ago

Something I'm used to do and I think is pretty good is creating functions with a positive default value

function showButton(isShown: bool = true) {
  // show or hide the button based on the isShown value
}

This way, the function calling is really explicit

showButton();
showButton(false);

This can apply to a lot of functions

ThaNarie commented 4 years ago

@selewi As a remark to your specific example, if the function name is an action, it feels super weird to pass a negative value.

showButton(false) would imply that it wouldn't "do the action", so the state would stay as it was.

It would be better to have picked setButtonVisiblity(). But calling that without a parameter would also be weird :)

I think it applies better to named (option) parameters, or props, like: <Foo isVisible={false}> - and leaving it out would mean it's visible.

Although that is weird for props that should be false 99% of the time, then you would end up with isInactive={true}.

selewi commented 4 years ago

@ThaNarie This is certainly true. The only thing I like is the fact I don't need two functions with almost the same code (doesn't always apply, of course).

showButton()
hideButton()

Can you think of any other way to avoid having this duplicated code and have still good reading functions? Using an interface as parameter could be an option but I don't like it a hundred percent to be honest.

setButtonState({ isVisible: true });
ThaNarie commented 4 years ago

@selewi In general I think I prefer to be explicit:

setButtonVisibility(true);
setButtonVisibility(false);

Which is what we most use in useState hooks anyway :)

There are also people who make a toggle function/hook (to be used without params), but adding the option of passing true or false explicitly, which would set that value. Which could be fine if toggling is the main use case, and explicitly setting it would only be done in a specific scenario. But in general (when not really using the toggle feature) i find it as confusing as your original example :)