1
\$\begingroup\$

Just wondering how I can go about doing this more efficiently/make it more visually appealing to the user?

     switch (this.state.selected) {
  case "all":
    styled = this.state.allKeywords.map(keyword => 
      <div style={{display:  "flex", flexDirection: "row"}}>
           <Input key={Math.random} style={{height: "45px", marginBottom: "20px"}} onClick={this.onKeywordClick} value={keyword} onChange={this.onKeywordChange}></Input>
           <Button key={Math.random} onClick={() => this.deleteKeyword(keyword)} style={{height: "45px", marginLeft: "30px"}}>Delete</Button>
      </div>
     );
    break;
  case "positive":
      styled = this.state.positive.map(keyword => 
        <div style={{display:  "flex", flexDirection: "row"}}>
             <Input key={Math.random} style={{height: "45px", marginBottom: "20px"}} onClick={this.onKeywordClick} value={keyword} onChange={this.onKeywordChange}></Input>
             <Button key={Math.random} onClick={() => this.deleteKeyword(keyword)} style={{height: "45px", marginLeft: "30px"}}>Delete</Button>
        </div>
       );  
     break;
  case "negative":
      styled = this.state.negative.map(keyword => 
        <div style={{display:  "flex", flexDirection: "row"}}>
             <Input key={Math.random} style={{height: "45px", marginBottom: "20px"}} onClick={this.onKeywordClick} value={keyword} onChange={this.onKeywordChange}></Input>
             <Button key={Math.random} onClick={() => this.deleteKeyword(keyword)} style={{height: "45px", marginLeft: "30px"}}>Delete</Button>
        </div>
       );
     break;
  default:
    console.log("No keyword list mapped");
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Repeated code—make it a function \$\endgroup\$ – D. Ben Knoble Oct 28 '19 at 3:18
  • \$\begingroup\$ The current question title, which states your concerns about the code, is too general to be useful here. Please edit to the site standard, which is for the title to simply state the task accomplished by the code. Please see How to get the best value out of Code Review: Asking Questions for guidance on writing good question titles. \$\endgroup\$ – BCdotWEB Oct 28 '19 at 11:34
2
\$\begingroup\$

If by this question you mean you want to prettify your code then Google "javascript prettify" and you will get numerous hits. For example, I picked the first one, Online JavaScript Beautifier and it did the following to your example with a few clicks. You can adjust the parameters of the prettifier to suit your needs. BTW this web site also mentions a Chrome plugin specifically aimed at prettifying code in Stack Overflow posts.

switch (this.state.selected) {
    case "all":
        styled = this.state.allKeywords.map(keyword =>
            <
            div style = {
                {
                    display: "flex",
                    flexDirection: "row"
                }
            } >
            <
            Input key = {
                Math.random
            }
            style = {
                {
                    height: "45px",
                    marginBottom: "20px"
                }
            }
            onClick = {
                this.onKeywordClick
            }
            value = {
                keyword
            }
            onChange = {
                this.onKeywordChange
            } > < /Input> <
            Button key = {
                Math.random
            }
            onClick = {
                () => this.deleteKeyword(keyword)
            }
            style = {
                {
                    height: "45px",
                    marginLeft: "30px"
                }
            } > Delete < /Button> <
            /div>
        );
        break;
    case "positive":
        styled = this.state.positive.map(keyword =>
            <
            div style = {
                {
                    display: "flex",
                    flexDirection: "row"
                }
            } >
            <
            Input key = {
                Math.random
            }
            style = {
                {
                    height: "45px",
                    marginBottom: "20px"
                }
            }
            onClick = {
                this.onKeywordClick
            }
            value = {
                keyword
            }
            onChange = {
                this.onKeywordChange
            } > < /Input> <
            Button key = {
                Math.random
            }
            onClick = {
                () => this.deleteKeyword(keyword)
            }
            style = {
                {
                    height: "45px",
                    marginLeft: "30px"
                }
            } > Delete < /Button> <
            /div>
        );
        break;
    case "negative":
        styled = this.state.negative.map(keyword =>
            <
            div style = {
                {
                    display: "flex",
                    flexDirection: "row"
                }
            } >
            <
            Input key = {
                Math.random
            }
            style = {
                {
                    height: "45px",
                    marginBottom: "20px"
                }
            }
            onClick = {
                this.onKeywordClick
            }
            value = {
                keyword
            }
            onChange = {
                this.onKeywordChange
            } > < /Input> <
            Button key = {
                Math.random
            }
            onClick = {
                () => this.deleteKeyword(keyword)
            }
            style = {
                {
                    height: "45px",
                    marginLeft: "30px"
                }
            } > Delete < /Button> <
            /div>
        );
        break;
    default:
        console.log("No keyword list mapped");
}
\$\endgroup\$
1
  • \$\begingroup\$ Hey, really appreciate this and will definitely put it to use. Although, is there a more efficient way to improve what my code actually does? \$\endgroup\$ – Rob Morrissey Oct 28 '19 at 4:05
1
\$\begingroup\$
  1. Simple change to improve would be to extract the repeated peace in separate component:
<div style={{display:  "flex", flexDirection: "row"}}>
 <Input key={Math.random} style={{height: "45px", marginBottom: "20px"}} onClick={this.onKeywordClick} value={keyword} onChange={this.onKeywordChange}></Input>
 <Button key={Math.random} onClick={() => this.deleteKeyword(keyword)} style={{height: "45px", marginLeft: "30px"}}>Delete</Button>
</div>
  1. You are incorrectly using key property on your components, I suggest you read the documentation on the keys in lists in react and see this stackoverflow question on why not use Math.random() as key.

You are also using key={Math.random} which is reference to the Math.random function rather than random value, though neither make sense.

  1. As mentioned in one of the comments and another answer, code style is much easier to maintain using automated tools. Most modern code editors support code formatting.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.