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>
 );
 }
}

• slepic: Please edit your question to include some details of what the code Is supposed to do. Also a brief description should go to the title. People Are more likely to want to review a progrese bar in react than they want to review random shapeless react code that they dont know what should be doing.
• BCdotWEB: Your title is too generic. It should state what your code does.
• Dheemanth Bhat: @BCdotWEB edited the question... if not good you can edit it as well.
• BCdotWEB: @Pure'ajax I don't know what your code does, you're supposed to tell us. 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.
• Sᴀᴍ Onᴇᴌᴀ: I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Feel free to edit and give it a different title if there is something more appropriate.

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 :)

• Roman: welcome @Orlyyn - keep up the good work
• Dheemanth Bhat: @Orlyyn So Whatever received in props, should it be updated to state variable in componentDidMount()?
• Orlyyn: Short answer: No, because componentDidMount does not fire when your component receives updated props, so if you get updated props (like changed props overtime), your component won't update. Instead, you could use componentDidUpdate. However it also depends, again, on what you want to do...

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"

• Orlyyn: Orlyyn is a "she" ;)
• m.sohail: haha. my bad will edit that. :)

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>
 );
 }
}