2
\$\begingroup\$

I am currently learning JS Frameworks, right now exploring React and wanted to make sure I keep everything in order, as in: the logic of working with React, code quality, any shortcomings, good/bad practices etc?

The code renders a simple Search panel that will allow to call an API, get the results (if any) and render the results on the page.

import React from "react";
import "./App.css";
import axios from "axios";

var Style = {
  marginRight: "22px"
};

var Style2 = {
  display: "none"
};

const API_CALL = "xxx";

class SearchForm extends React.Component {
  constructor(props) {
    super(props);
    this.state = {
      value: "",
      errorValue: "",
      countryCode: "",
      VATNumber: "",
      valid: "",
      name: "",
      address: "",
      isLoading: false,
      isSubmitted: false
    };

    this.handleChange = this.handleChange.bind(this);
    this.handleSubmit = this.handleSubmit.bind(this);
  }

  renderField() {
    return (
      <form onSubmit={this.handleSubmit}>
        <label style={Style}>Search VAT:</label>
        <input type="text" onChange={this.handleChange} style={Style} />
        <input type="submit" value="Submit" />
      </form>
    );
  }

  render() {
    return (
      <div>
        <h5>Reference: EE100247019</h5>
        {this.renderField()}
        <div id="results" />
        <Loader loading={this.state.isLoading} />
        {this.state.isSubmitted && (
          <Result
            country={this.state.countryCode}
            number={this.state.VATNumber}
            name={this.state.name}
            address={this.state.address}
            error={this.state.errorValue}
            valid={this.state.valid}
            loading={this.state.isLoading}
          />
        )}
      </div>
    );
  }

  handleChange(event) {
    this.setState({ value: event.target.value.trim() });
  }

  handleSubmit(event) {
    this.setState({ isLoading: true, isSubmitted: false });
    Style2 = {
      listStyleType: "none",
      textAlign: "left",
      display: "block",
      border: "1px solid white",
      marginTop: "50px"
    };
    axios
      .get(API_CALL + this.state.value)
      .then(res =>
        this.setState({
          countryCode: res.data.CountryCode,
          VATNumber: res.data.VATNumber,
          name: res.data.Name,
          address: res.data.Address,
          valid: res.data.Valid,
          isLoading: false,
          isSubmitted: true
        })
      )
      .catch(error =>
        this.setState({
          valid: false,
          errorValue: this.state.value,
          isLoading: false,
          isSubmitted: true
        })
      );

    event.preventDefault();
  }
}

function Loader(props) {
  if (!props.loading) {
    return null;
  }
  return <h6> Loading ... </h6>;
}

function Result(props) {
  if (!props.valid) {
    return (
      <h5>
        Invalid value "{props.error}"
        <br /> <br />
        Please enter valid VAT Number
      </h5>
    );
  } else {
    return (
      <table style={Style2}>
        <tr>
          <td>Country code: </td>
          <td>{props.country}</td>
        </tr>
        <tr>
          <td>VAT Number: </td>
          <td>{props.number}</td>
        </tr>
        <tr>
          <td>Product name: </td>
          <td>{props.name}</td>
        </tr>
        <td>Address: </td>
        <td>{props.address} </td>
      </table>
    );
  }
}

export default SearchForm;
\$\endgroup\$
1
  • \$\begingroup\$ Looks OK; Code sample can be much shorter for this question; Learn react hooks, they are awesome :) \$\endgroup\$
    – user134596
    Jul 11, 2019 at 6:35

1 Answer 1

1
\$\begingroup\$

Here is the code after refactor some changes that i think should be good to follow:

  1. use let instead of var.
  2. I have used style2 as a state as its changing.
  3. Used arrow function instead of bind.
  4. Prefer destructuring.
import React from "react";
import "./App.css";
import axios from "axios";

let Style = {
  marginRight: "22px"
};

const API_CALL = "xxx";

class SearchForm extends React.Component {
  constructor(props) {
    super(props);
    this.state = {
      value: "",
      errorValue: "",
      countryCode: "",
      VATNumber: "",
      valid: false,
      name: "",
      address: "",
      isLoading: false,
      isSubmitted: false,
      Style2: { display: "none" }
    };
  }

  renderField() {
    return (
      <form onSubmit={this.handleSubmit}>
        <label style={Style}>Search VAT:</label>
        <input type="text" onChange={this.handleChange} style={Style} />
        <input type="submit" value="Submit" />
      </form>
    );
  }

  render() {
    const {
      Style2 = {},
      countryCode = "",
      VATNumber = "",
      valid = false,
      name = "",
      address = "",
      isLoading = false,
      isSubmitted = false,
      errorValue = ""
    } = this.state;
    return (
      <div>
        <h5>Reference: EE100247019</h5>
        {this.renderField()}
        <div id="results" />
        <Loader loading={isLoading} />
        {isSubmitted && (
          <Result
            country={countryCode}
            number={VATNumber}
            name={name}
            address={address}
            error={errorValue}
            valid={valid}
            loading={isLoading}
            style={Style2}
          />
        )}
      </div>
    );
  }

  handleChange = event => this.setState({ value: event.target.value.trim() });

  handleSubmit = event => {
    this.setState(({ Style2 }) => ({
      isLoading: true,
      isSubmitted: false,
      Style2: {
        ...Style2,
        listStyleType: "none",
        textAlign: "left",
        display: "block",
        border: "1px solid white",
        marginTop: "50px"
      }
    }));
    axios
      .get(API_CALL + this.state.value)
      .then(
        ({
          data: {
            CountryCode = "",
            VATNumber = "",
            Name = "",
            Address = "",
            Valid = false
          } = {}
        } = {}) =>
          this.setState({
            countryCode: CountryCode,
            VATNumber: VATNumber,
            name: Name,
            address: Address,
            valid: Valid,
            isLoading: false,
            isSubmitted: true
          })
      )
      .catch(error =>
        this.setState({
          valid: false,
          errorValue: this.state.value,
          isLoading: false,
          isSubmitted: true
        })
      );

    event.preventDefault();
  };
}


const Loader = ({ loading = false } = {}) =>
  loading ? null : <h6> Loading ... </h6>;

const Result = ({
  valid = false,
  error = "",
  style = {},
  country = "",
  number = "",
  name = "",
  address = ""
} = {}) => {
  if (!valid) {
    return (
      <h5>
        Invalid value "{error}"
        <br /> <br />
        Please enter valid VAT Number
      </h5>
    );
  } else {
    return (
      <table style={style}>
        <tr>
          <td>Country code: </td>
          <td>{country}</td>
        </tr>
        <tr>
          <td>VAT Number: </td>
          <td>{number}</td>
        </tr>
        <tr>
          <td>Product name: </td>
          <td>{name}</td>
        </tr>
        <td>Address: </td>
        <td>{address} </td>
      </table>
    );
  }
}


export default SearchForm;
\$\endgroup\$
2
  • \$\begingroup\$ Hi, you should explain why the changes you made to the code makes it better. \$\endgroup\$
    – IEatBagels
    Jul 19, 2019 at 15:21
  • \$\begingroup\$ the code follows the best practices that we should follow as asked by @Altair312 in the quesion \$\endgroup\$ Jul 19, 2019 at 17:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.