0
\$\begingroup\$

I'm new to React and started this project as a sandbox to experiment with it. I implemented a main page wich has a header and some content. The content is selected on the header. I'm wondering how it stands on the react best practices and other ways it could be implemented.

On the main app, we hold as state the index of the content we want to show, and we pass to the header as a property a reference to the funtion that handles the index state, like so

 class App extends Component {
  constructor() {
    super();
    this.state = {
      cidx: 0,
      //content:[<Heroes/>, <Teams/>]
    }

    this.updateContent = this.updateContent.bind(this);
    this.getContent = this.getContent.bind(this);
  }

  updateContent(idx) {
    this.setState({cidx:idx});
  }

  getContent() {
    //return this.state.content[this.state.cidx];
    switch(this.state.cidx) {
      case 0:
        return <Heroes/>
      case 1: 
        return <Teams/>
    }
  }

  render() {
    const Content = this.getContent();
    return (
      <div>
        <Header updateContent={this.updateContent} textItems={["Heroes", "Teams"]}/>
        {Content}
      </div>
    );
  }
}

export default App;

Like that we can inject the content in our render function based on user interaction with the header. I used a switch statement in getContent to avoid having an array of objects in memory all the time. Is it actually better performance-wise ?

This is the header code

class Header extends Component {
  constructor(props) {
    super(props);
  }

  render() {
    const items = this.props.textItems.map(
      (item, index)=>{
        return <NavItem key={index} onClick={()=>{this.props.updateContent(index)}}>{item}</NavItem>
      }
    );
    return (
      <div className="App">
        <Navbar className="deep-purple" brand='Dota Wizard' right>
          {items}
        </Navbar>
      </div>
    );
  }
}

export default Header;

A working version of the project is found on https://glitch.com/~absorbing-buzzard

\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review. As far as I can see, there are some JSX elements missing at the moment, for example Navbar, Heroes and Teams. Even if they're not required, a short description of those can help reviewers. \$\endgroup\$ – Zeta Dec 22 '18 at 8:59
0
\$\begingroup\$

Quick review.

This is a very basic review as I am not a react fan. Do not accept this post as an answer or you will likely dissuade others from adding a better one.

Your question

"I used a switch statement in getContent to avoid having an array of objects in memory all the time. Is it actually better performance-wise ?"

By "better performance-wise" I assume you mean CPU cycles. The answer is, "No its not better."

Computation complexity

For 2 items the difference is tiny, but if you had a long list of items say n is number of items. To display the last item would require n comparisons so worst case is O(n), and best case display the first item is O(1). Picking random items the complexity is O((log n)c) with c just above 2 (AKA polylogarithmic)

What you had return this.state.content[this.state.cidx];, basicly a lookup table, is O(1) the best performance you can get.

Storage complexity

Many times there is a price to pay for using a lookup table and that is memory. But in this case that makes no difference as you are after already stored items rather than looking up a precomputed value.

As I understand it the react elements you defined as <Heroes/>, <Teams/> are store in an internal node tree, having them in an array means you have an extra item in the state object to reference the array and one reference per item. That would require storage O(2n + 1) rather than O(n). Where n in this case refers to a reference to the react elements, not the elements themselves.

O(2n+1) and O(n) are the same, and tiny compared to the elements actual needs.

Style

You could make a slight change

  updateContent(idx) {
    this.setState({cidx:idx});
  }

to

  updateContent(cidx) {
    this.setState({cidx});
  }

But really I only say so as filler as your code looks good, like its almost a cut and paste from the best of react, a good thing (I am not implying you cut and pasted anything 🙂)

I do have a long list of style points in regard to your code, but as that list directly contradicts the react style guide it would be rather bad advice.

Merry Xmas

\$\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.