Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feedback: boolean logic #141

Open
jackherizsmith opened this issue Nov 15, 2023 · 0 comments
Open

Feedback: boolean logic #141

jackherizsmith opened this issue Nov 15, 2023 · 0 comments

Comments

@jackherizsmith
Copy link
Contributor

jackherizsmith commented Nov 15, 2023

I'm just using this page as an example because it's in front of me - show is not only already truthy or falsy, it's literally true or false. So for example can be simplified:

        {show ? 'Show Less' : 'Show More'}

I have some opinions about using booleans, i.e. ⚠️ opinion alert ⚠️ Here they are:

Naming booleans

A great name for a boolean is telling the developer what is currently true when the boolean is true. For example, this variable show could be isShowingAll. Maybe it feels more verbose, and to be honest show here is not a great example of my point. But "show" is a verb, and booleans represent the state of something as it is currently. A better example might be if you have a variable that is true when a date is today - you could have a variable today which is true, or isToday and it's just a bit clearer, not only about what it stands for but also the type of variable it is (because today could also be a date). An added advantage is slightly more readable if blocks, i.e. if(isShowingAll) just reads a bit better (in my opinion).


use ? more

I might be missing something but I'm seeing a lot of user && user.id, and place && place[0].coordinates, is there a reason you aren't using user?.id and place?.[0].coordinate?

something like

place && place.length > 0 ? (
  <>
    <DisplayPlaceCard...

can be simplified to

place?.length ? (
  <>
    <DisplayPlaceCard...

if you like, although this is definitely a preference thing - to be honest I quite like the explicitness of how you've written it. I suppose just so long as it's a conscious decision to be more verbose in order to be clear about what is happening 👍

found another example

    if (
      wishListArr &&
      wishListArr[0]['wish_list'] &&
      wishListArr[0]['wish_list'].includes(id)
    )

could be just

    if (wishListArr?.[0]['wish_list']?.includes(id)) return; // or something similar

that being said! I personally would put this in a variable using the approach above, to make the if block really clear:

    const isInWishlist = !!wishListArr?.[0]['wish_list']?.includes(id);
    if (isInWishlist) return;

Aim for truthy variables

aim for truthy statements, I often see something like

if (!notShowing) {}

which is confusing. If your boolean variable is always named truthily then your usage of it will always avoid double negatives. Also sometimes I see

if (!notShowing) {} else {}

and clearly they can just be swapped, so the true state is always addressed first, so overall

if (isShowing) {} else {}

Don't use if blocks to set boolean state

when setting a variable to be a boolean conditionally, use the condition in the setting of it. here's an example I see a lot

if (visibleItemsCount === itemsCount) { // fun fact! I think variables that represent numbers of stuff should be stuffCount
  setIsShowingAll(true);
} else {
  setIsShowingAll(false)
}

can actually just be

setIsShowingAll(visibleItemsCount === itemsCount);

let's say you have a bunch of parameters for that to be true, I like to set them out as variables first:

const canSeeAllItems = visibleItemsCount === itemsCount; // this is a bad example but when there's a few conditions to combine it makes things much clearer
setIsShowingAll(canSeeAllItems);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant