batch progress visualization In React.js

Question

I am slowly digesting React.js, found couple of ways to write the same piece of code: Please tell where all I went wrong in below code.

1. I try to put logic inside render(), is that Ok?
2. Not using React life cycles for simple components?
3. Use Functional components instead of class components?

File 1: batch_progress.js

Description: Shows batch progress, expects 3 values:

1. current batch.
2. target batch count.
3. state of the batch (based on which the progress bar color changes).

import React, { Component } from 'react'; import ProgressBar from '../progress_bar/progress_bar'; export default class BatchStatus extends Component { constructor(props) { super(props); } render() { let color; let percentage = (this.props.batch.number / this.props.targetBatchCount) * 100; switch (this.props.batch.status) { case 100: color = '#e7e4f1'; break; case 200: color = '#c3dcec'; break; case 300: color = '#ecc6eb'; break; case 400: color = '#ecdec3'; break; case 500: color = '#c8ecc7'; break; default: color = '#e7e4f1'; } return ( <ProgressBar foregroundColor={color} percentage={percentage}> {this.props.batch.number}&nbsp;/&nbsp;{this.props.targetBatchCount} </ProgressBar> ); } } 

File 2: progress_bar.js

import React, { Component } from 'react'; import './progress_bar.css'; export default class ProgressBar extends Component { constructor(props) { super(props); } render() { let foregroundColor = this.props.foregroundColor || '#e7e4f1'; let percentage = this.props.percentage || 0; let backgroundColor = this.props.backgroundColor || '#eceeef'; let style = { backgroundImage: 'linear-gradient(to right, ' + foregroundColor + ' ' + percentage + '%, ' + backgroundColor + ' 0%)' }; return ( <div className="progress-bar-container" style={style}> {this.props.children} </div> ); } } 

Answer 1

The function render() in only supposed to render. It is not supposed to compute some style, set variables, have any logic other than render: (Functions Names Should Say What They Do). This should respect the SRP (Single Responsiblity Principle), as well as another clean code rule: 'A Function Should Do One Thing'.

What could be code smell in your first file:

• A unused constructor should not be there at all,
• The percentage calculation should be put in a function of its own
• The color choice should be placed in a function of its own

What could be code smell in your second file:

• A unused constructor should be removed

For both files, you could also add some prop types verifications using the library 'prop_types'.

As for your questions, with short answers:

1. Not it's not Ok
2. Depends on what you intend to do!
3. This article should help you choose: React Functional or Class components ? - even though you can use hooks now as well :)

Answer 2

The answer by Orlyyn explains it well. Just adding to it:

• You can probably destructure your variables, this helps with readability and reuse.
• Since you are not using the state of the class you can convert that into Functional Component
• Use template string instead of doing it in bits.
• Still no need to add this.state and call setState in your componentDidMount(). What Orlyyn meant by single responsibility principle was that there shouldn't be any other logic in there, simply. You can still get the variables that you need for that function to work.

The State is useful when you need to manipulate it within your very class or function.

So, a refactor of your code batch_progress.js could be as following:

import React from 'react'; import PropTypes from 'prop-types'; import ProgressBar from '../progress_bar/progress_bar'; const statusCodeColors = { 100: '#e7e4f1', 200: '#c3dcec', 300: '#ecc6eb', 400: '#ecdec3', 500: '#c8ecc7', }; const getStatusColor = (code) => statusCodeColors[code] || '#e7e4f1'; const getPercentage = (num, total) => (num / total) * 100; const BatchStatus = ({ batch: { number, status }, targetBatchCount }) => { const color = getStatusColor(status); const percentage = getPercentage(number, targetBatchCount); return ( <ProgressBar foregroundColor={color} percentage={percentage}> {`${number} / ${targetBatchCount}`} </ProgressBar> ); }; BatchStatus.defaultProps = { batch: { number: 50, status: 100, }, targetBatchCount: 100, }; BatchStatus.propTypes = { batch: PropTypes.shape({ number: PropTypes.number, status: PropTypes.number, }), targetBatchCount: PropTypes.number, }; export default BatchStatus; 

Included the prop-types there. Might help you in getting started with it. Though react has a great documentation for it.

I have also replaced your switch-case statement there with something more cleaner. You can read about it in following articles:

• "Probably the hottest code refactor..."
• "Keeping your code clean by sweeping out "if" statements"

Answer 3

File 1: batch_progress.js

import React, { Component } from 'react'; import ProgressBar from '../progress_bar/progress_bar'; export default class BatchStatus extends Component { constructor(props) { super(props); this.state = { color: '' } } componentDidUpdate = (prevProps, prevState) => { if (this.props === prevProps) { return; } this.buildProgressBar(); } componentDidMount = () => { this.buildProgressBar(); } buildProgressBar = () => { let color; let percentage = (this.props.batch.number / this.props.targetBatchCount) * 100; switch (this.props.batch.status) { case 100: color = '#e7e4f1'; break; case 200: color = '#c3dcec'; break; case 300: color = '#ecc6eb'; break; case 400: color = '#ecdec3'; break; case 500: color = '#c8ecc7'; break; default: color = '#e7e4f1'; } this.setState({ color: color, percentage: percentage }) } render() { return ( <ProgressBar foregroundColor={this.state.color} percentage={this.state.percentage}> {this.props.batch.number}&nbsp;/&nbsp;{this.props.targetBatchCount} </ProgressBar> ); } } 

File 2: progress_bar.js

import React, { Component } from 'react'; import './progress_bar.css'; export default class ProgressBar extends Component { render() { let foregroundColor = this.props.foregroundColor || '#e7e4f1'; let percentage = this.props.percentage || 0; let backgroundColor = this.props.backgroundColor || '#eceeef'; let style = { backgroundImage: 'linear-gradient(to right, ' + foregroundColor + ' ' + percentage + '%, ' + backgroundColor + ' 0%)' }; return ( <div className="progress-bar-container" style={style}> {this.props.children} </div> ); } }