Reputation: 127
I'm doing a calculator in React and rendering all the buttons through the Button Panel component using "brute force".
return (
<>
<div>
<Button value="A/C" clickHandler={handleClick} />
<Button value="+/-" clickHandler={handleClick} />
<Button value="%" clickHandler={handleClick} />
<Button value="÷" clickHandler={handleClick} />
</div>
<div>
<Button value="1" clickHandler={handleClick} />
<Button value="2" clickHandler={handleClick} />
<Button value="3" clickHandler={handleClick} />
<Button value="x" clickHandler={handleClick} />
</div>
<div>
<Button value="4" clickHandler={handleClick} />
<Button value="5" clickHandler={handleClick} />
<Button value="6" clickHandler={handleClick} />
<Button value="+" clickHandler={handleClick} />
</div>
<div>
<Button value="7" clickHandler={handleClick} />
<Button value="8" clickHandler={handleClick} />
<Button value="9" clickHandler={handleClick} />
<Button value="-" clickHandler={handleClick} />
</div>
<div>
<Button value="0" clickHandler={handleClick} />
<Button value="." clickHandler={handleClick} />
<Button value="=" clickHandler={handleClick} />
</div>
</>
);
But I realized that I was repeating too much code, so I wrote the following structure.
const symbols = [
'A/C', '+/-', '%', '÷',
'1', '2', '3', 'x',
'4', '5', '6', '+',
'7', '8', '9', '-',
'0', '.', '=',
];
return (
<>
<div>
{symbols.map((s) => <Button value={s} key={s} clickHandler={handleClick} />)}
</div>
</>
);
So... in my opinion, the first solution makes it better to style the buttons with individual aesthetics and also separate them into unique rows, but I feel like I'm not following the DRY practice. Which approach is the best when it comes to making it easier to style and structure it? (My goal is that every number is grey and the operators are orange.) Should I find a balance?
Is there a better way to render multiple components that just the value passed to it is different?
How can I improve my code?
Upvotes: 1
Views: 925
Reputation: 744
You can create a new Row component, which render the buttons on one row.
const Row = (props) => {
return (
<div>
{
props.buttons.map((button) => <button value=button.value onClick={() =>
props.handleClick(button.value)} className={button.className}/>
}
</div>
)
}
You can render the row component to form a number pad.
const cal = (props) => {
const buttons = [
[{value: 'A/C', className: 'A/C'}, {value: '+/-', className: '+/-'}, {value: '%', className: '%'}, {value: '/', className: '/'}],
[{value: '1', className: '1'}, {value: '2', className: '2'}, {value: '3', className: '3'}, {value: 'x', className: 'x'}],
[{value: '4', className: '4'}, {value: '5', className: '5'}, {value: '6', className: '6'}, {value: '+'}, className: '+'],
[{value: '7', className: '7'}, {value: '8', className: '8'}, {value: '9', className: '9'}, {value: '-', className: '-'}],
[{value: '0', className: '0'}, {value: '.', className: '.'}, {value: '=', className: '='}]
]
const handleClick = (value) => { // add logic with switch cases/ if else}
//....
return (
{
list.map((row) => {
return <Row buttons=[buttons] handleClick={handleClick}/>
})
})
//....
}
With this approach if you are going the create a new scientific calculator in future then you can use the same component to extent the calculator.
Upvotes: 0
Reputation: 449
NOTE: This is all entirely opinion.
Your primary concern when writing code should be "is this code clear and understandable?" I can't tell you how many times I've come back months later to code I wrote and thought "what in the world is this?"
With that in mind, DRY is mainly useful when considering reuse and refactoring. Reuse: "Would other people benefit from reusing the thing I just wrote?" Refactor: "Would refactoring all instances of this code be a pain and might I miss some of it?" If either of those are yes, it should probably be DRY.
In your case, I believe that the answers to both are 'No'. All uses are in one spot, so pretty hard to miss. People probably aren't going to reuse this component either.
Regarding styling, imagine the future where you decide that certain buttons use unique styling (such as spanning multiple "cells"), mapping through a list of symbols and numbers is going to make that harder.
That being said, you can definitely make it clearer. For example, you could write more specific components:
const NumberButton = (props) => {
return <Button className={'orange-button'} {...props} />
}
const SymbolButton = (props) => {
return <Button className={'orange-button'} {...props} />
}
These component might be a) used by another dev or in another part of the code-base and b) make it very clear what they should be used for.
In other words, take pity on your future self and/or other devs. Write really dumb code that's easy to read.
Upvotes: 0
Reputation: 2090
I think in order to be more spesific, you need to create an array of objects which carries more informations and allows you to do things conditionally. First thing came into my mind is something like this;
const buttonArr = [
{
symbol: "1",
type: "num",
},
{
symbol: "2",
type: "num",
},
{
symbol: "A/C",
type: "operator",
},
];
then you can render something like this;
const buttons = () => {
return (
<div>
{buttonArr.map((button) => {
<button
style={{ backgroundColor: button.type === "num" ? "grey" : "orange" }}
>
{button.symbol}
</button>;
})}
</div>
);
};
This is a simple example, so in general my advice is populate the buttons array more if you want if necessary to conditionally do stuff. You can maybe seperate the operator array from number array so you can put them different divs if you want.
Upvotes: 1