0

Following is my code (which is working fine) in which I am able to sort list on the basis of input provided in text box. In constructor method I declared my states like this -

this.state = {
      data: ["Adventure", "Romance", "Comedy", "Drama"],
      tableData: []
    };

In componentDidMount method I assign the data key state in tableData.

  componentDidMount() {
    this.setState({
      tableData: this.state.data
    });
  }

My question is - Is it the correct way of doing this as somehow I myself not feeling confident about this code quality (Initializing the tableData as [] and then setting tableData: this.state.data in componentDidMount method ). Let me know if I can improve this also what will change if I fetch the data from an API which is the best place to initialize and use in an app.

Working Code Example - https://codesandbox.io/s/k9j86ylo4o

Code -

class App extends Component {
  constructor(props) {
    super(props);
    this.state = {
      data: ["Adventure", "Romance", "Comedy", "Drama"]
    };
    this.handleChange = this.handleChange.bind(this);
  }

  refineDataList(inputValue) {
    const listData = this.state.data;
    const result = listData.filter(item =>
      item.toLowerCase().match(inputValue.toLowerCase())
    );
    this.setState({
      data: result
    });
  }

  handleChange(e) {
    const inputValue = e && e.target && e.target.value;
    this.refineDataList(inputValue);
  }
  render() {
    return (
      <div className="App">
        <h3>DATA SEARCH</h3>
        <div className="form">
          <input type="text" onChange={this.handleChange} />
        </div>
        <div className="result">
          <ul>
            {this.state.data &&
              this.state.data.map((item, i) => {
                return <li key={i}>{item}</li>;
              })}
          </ul>
        </div>
      </div>
    );
  }
}
Nesh
  • 2,389
  • 7
  • 33
  • 54
  • 1
    Why do you need both data and tableData if you set the later to the value of the first one? Why not remove data from the state and set tableData to the initial values? And regarding fetch, if you fetch only once then componentDidMount is a good place to do it. – Yossi Apr 20 '19 at 05:15
  • @Yossi I tried this with single source of data, but the problem is on filtering the this.state.data gets changed as well, so I need another variable to store the filtered data as well – Nesh Apr 20 '19 at 05:28
  • I don't understand. Can you post the entire component? – Yossi Apr 20 '19 at 05:31
  • @Yossi check this code - https://codesandbox.io/s/k9j86ylo4o – Nesh Apr 20 '19 at 05:42
  • Please post it here, it will be more convenient for me, and also easier to refer to pieces of code, if needed – Yossi Apr 20 '19 at 05:46
  • @Yossi Done..I changed this ..to use only one state variable now instead of two – Nesh Apr 20 '19 at 05:48
  • Thanks. I see that somebody already posted an answer, so I will let him help you. If you still have any question, please write another comment. – Yossi Apr 20 '19 at 05:56

1 Answers1

1

You are doing great, but you are right there is a way to do it in better way, handle two point of truth is difficult to maintain, so you should have only one data array with the words that you need, so the way you should filter the values is by creating a filter variable into state to store the current word to be filtered, so you should add something like

// in the constructor function
constructor(props) {
  super(props);
  this.state = {
    data: ["Adventure", "Romance", "Comedy", "Drama"],
    filter: ""
  }
}

// create a filter function
getFilteredResults() {
  const { filter, data } = this.state;
  return data.filter(word => String(word).toLowerCase().match(filter));
}

// and finally into your render function
render() {
  return (
    <div>
      {this.getFilteredResults().map((word) => (
        <div>{word}</div>
      ))}
    </div>
  );
}

Obviously remember to update your handleChange function, like so

handleChange(e) {
  const inputValue = e && e.target && e.target.value;
  this.setState({ filter: inputValue });
  //this.refineDataList(inputValue);
}

In that way you will maintain only one point of truth, it will work as expected.

Note: we use String(word).toLowerCase() to ensure that the current word is actually a string, so we can avoid the toLowerCase is not function of undefined error, if for some reason word is not a string.

Juorder Gonzalez
  • 1,642
  • 1
  • 8
  • 10