Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

blog/no-love-for-boolean-parameters #17

Closed
utterances-bot opened this issue Feb 28, 2021 · 6 comments
Closed

blog/no-love-for-boolean-parameters #17

utterances-bot opened this issue Feb 28, 2021 · 6 comments

Comments

@utterances-bot
Copy link

No love for boolean parameters | TkDodo's blog

Innocent looking boolean parameters, or flags, are often the reason for hard to maintain legacy code. Resist the urge of adding them!

https://tkdodo.eu/blog/no-love-for-boolean-parameters

Copy link

This is great advice. I am totally guilty of that button use case. Gonna go change that right now to variant instead! Thanks!!

Copy link

Very nice article. It inspired me to rethink table components in react as I've often come across table with an isLoading prop.
Thanks to your article, I now prefer:

<PostsTable
        posts={data}
        setFilters={setFilters}
        columns={columns}
        status={tableStatus}
      />
import {FancyTableHeaders} from "./headers";

interface FancyTableProps {
	status: "success" | "empty" | "error" | "loading" | "idle";
}
function FancyTable ({status}: FancyTableProps) {
	
	return (
    <table>
      <FancyTableHeaders
        variant={status === "success" ? "enabled" : "disabled"}
      />
      <tbody>
			 {/* ✅ little extra boilerplate */}
       {/* ✅ clear conditional rendering */}        
			 {
          {
            success: <PostsTableBody rows={rows} prepareRow={prepareRow} />,
            empty: <EmptyTableBody />,
            error: <ErrorTableBody />,
            loading: <LoadingTableBody />,
            idle: <IdleTableBody />
          }[status]
        }
      </tbody>
    </table>
  );
}

Copy link

I couldn't find the edit comment option, so:
Avoid multiple boolean props

Copy link

Really nice article! The only change I would do is change this switch to a object, but it's only a opinion. I think is more readable and clean:

const variants = {
    percent: (value: number): string => `${value}%`, 
    currency: (value: number): string => `ur method - R$${value}` , 
    standard: (value: number): string => String(value) 
}

function formatMetric(
  value: number,
  variant: keyof typeof variants = 'standard'
): string {
  return variants[variant](value);
}

What do you think?

@TkDodo
Copy link
Owner

TkDodo commented Sep 18, 2021

@eusousalvi yes, that's another way, and it nicely ties together types (keyof typeof variants) and the implementation. This works best if you can actually derive the type from there, which might not work if the types are defined somewhere else (in a contract or so). Then, you have to make sure that variants conforms to that, but the differences are really minor :)

Copy link

Thank you so much for this article. I had a lot of "oh" moments as i read the article. Very insightful

Repository owner locked and limited conversation to collaborators Apr 15, 2022
@TkDodo TkDodo converted this issue into discussion #56 Apr 15, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants