1
\$\begingroup\$

I'm pretty new to React and I was working on some JSON data. Now I just to understand basic concepts, so I wrote this little component, but in my eyes it's not efficient.

import React from 'react';
import {getJson} from './getJson';
import AddRow from './AddRow';

class TableComponent extends React.Component {
    constructor(props) {
      super(props)
      this.state = {
        checked: true,
        rows:[],
        json: []
      }
    }

    checkboxHandler() {}

    componentDidMount() {
      this.setState((prevState) => {
        return {
          json: getJson(),
        }
      })
    }

    checkAll = () => {
        this.setState({
            checked: !this.state.checked
        })

        this.setState(prevState => {
          const json = prevState.json.map(obj => ({
            ...obj,
            items: obj.items.map(item => ({
              ...item,
              value: this.state.checked,
            }))
          }));
          return { json };
        });
      };

    render() {
      return (
        <div>
          <table>
            <tbody>
              {this.state.json.map((obj, i) => {
                return (
                  <tr key={obj.id}>
                    {obj.items.map((data, i) => {
                        return( 
                            <td key={data.id}> 
                                <p>{data.label}</p>
                                    <input 
                                        type="checkbox" 
                                        checked={data.value}
                                        onChange={this.checkboxHandler}
                                    />
                            </td>
                        )
                    })}
                  </tr>
                )
              })}
            </tbody>
          </table>
          <button onClick={this.checkAll}>check\uncheck</button>
          <AddRow actualJson={this.state.json}/>
        </div>
      )
    }
  }

  export default TableComponent;

I'm not proud of it because I feel that I'm not respecting the heart of React - use module and components. Is there a way to have cleaner code?

\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! (Something's wrong with the first part of your second sentence.) Can you add more of what the code is to accomplish to the title and/or the question? \$\endgroup\$ – greybeard Feb 17 at 15:47
0
\$\begingroup\$

There isn't much code to break this down to smaller components but based on the post :

this.setState() passing an Object or a callback function :

If you want to update the state with a value that's not dependent on the previous one you can do :

this.setState({ key : newValue });

If you have to update the state based on the previous one,

this.setState(prevState => ({key: prevState.key + value }));

see state updates may be asynchronous for more details.

the reason why i mention this is because in componentDidMount() you're passing a callback function to setState when you don't need the the prevState.

componentDidMount() {
  this.setState({ json: getJson() })
}

In checkAll() you can do one setState() instead of two:

checkAll = () => {
   this.setState(prevState => ({
     checked: !prevState.checked,
     json: prevState.json.map(obj => ({
       ...obj,
       items: obj.items.map(item => ({
         ...item,
         value: prevState.checked,
       }))
     }))
   }));
};

Use implicit return in the render method :

the render method can use implict returns inthe .map() and you don't really need the i, it can be refactored to :

render() {
  return (
    <div>
      <table>
        <tbody>
          {this.state.json.map(obj => (
            <tr key={obj.id}>
              {obj.items.map(data => ( 
                <td key={data.id}> 
                  <p>{data.label}</p>
                    <input 
                      type="checkbox" 
                      checked={data.value}
                      onChange={this.checkboxHandler}
                    />
                </td>)
              )}
            </tr>)
          )}
        </tbody>
      </table>
      <button onClick={this.checkAll}>check\uncheck</button>
      <AddRow actualJson={this.state.json}/>
    </div>
  )
}
\$\endgroup\$
  • 1
    \$\begingroup\$ thanks for the feedback :) i will study it! \$\endgroup\$ – Legeo Feb 17 at 22:46

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.