@1233211

Как можно улучшить этот компонент?

Подскажите, что в этом коде можно было бы улучшить и какие тут могут быть или есть проблемы
Спасибо

import React, { Component } from "react";
import { getUsers, getOrganizations } from "./api";

class App extends Component {
  state = {
    loading: true,
    selectedOrg: null
  };
  users = [];
  organizations = [];
  componentDidMount() {
    getUsers()
      .then((users) => (this.users = users))
      .then(() => getOrganizations())
      .then((organizations) => (this.organizations = organizations))
      .then(() => this.setState({ loading: false }));
  }

  selectOrg = (org) => {
    this.setState({ selectedOrg: org });
  };

  resetSelectedOrg = () => {
    this.setState({ selectedOrg: false });
  };

  render() {
    if (this.state.loading) {
      return "Loading...";
    }

    let users = [];

    for (let i = 0; i < this.users.length; i++) {
      const name = this.users[i].name;
      let org;

      for (let j = 0; j < this.organizations.length; j++) {
        if (this.organizations[j].id === this.users[i].organization) {
          org = this.organizations[j].name;
        }
      }

      users.push(
        <div className="user-list-item">
          <div>name: {name}</div>
          <div onClick={() => this.selectOrg(org)}>org: {org}</div>
        </div>
      );
    }

    if (this.state.selectedOrg) {
      users = [];
      for (let i = 0; i < this.users.length; i++) {
        const orgId = this.organizations.find(
          (o) => o.name === this.state.selectedOrg
        ).id;

        if (this.users[i].organization === orgId) {
          users.push(
            <div className="user-list-item">
              <div>name: {this.users[i].name}</div>
              <div>org: {this.state.selectedOrg}</div>
            </div>
          );
        }
      }
    }

    return (
      <div>
        {this.state.selectedOrg && (
          <button onClick={() => this.resetSelectedOrg()}>
            reset selected org
          </button>
        )}
        <div className="user-list">{users}</div>
      </div>
    );
  }
}

export default App;
  • Вопрос задан
  • 109 просмотров
Решения вопроса 1
Dasihub
@Dasihub

getUsers()
            .then((users) => (this.users = users))
            .then(() => getOrganizations())
            .then((organizations) => (this.organizations = organizations))
            .then(() => this.setState({ loading: false }));


Вот здесь место then лучше использовать await и try catch.


if (this.state.loading) {
      return "Loading...";
    }

    let users = [];

    for (let i = 0; i < this.users.length; i++) {
      const name = this.users[i].name;
      let org;

      for (let j = 0; j < this.organizations.length; j++) {
        if (this.organizations[j].id === this.users[i].organization) {
          org = this.organizations[j].name;
        }
      }

      users.push(
        <div className="user-list-item">
          <div>name: {name}</div>
          <div onClick={() => this.selectOrg(org)}>org: {org}</div>
        </div>
      );
    }


Вот здесь переменные всегда должны быть сверху выше всех


{this.state.selectedOrg && <button onClick={() => this.resetSelectedOrg()}>reset selected org</button>}


Не стоит создавать анонимные функции это плохо влияет на React


for (let j = 0; j < this.organizations.length; j++) {
        if (this.organizations[j].id === this.users[i].organization) {
          org = this.organizations[j].name;
        }
      }

Я одно понять не могу зачем делать такие циклы и использовать как перебор массива, когда есть
for (let i of arr)
Работает как у тебя, но кода будет чище

В целом компомнент не плохой, но написан на классовом компоненте, лучше писать на функиональном компоненте кода будет намного чище
Ответ написан
Пригласить эксперта
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Войти через центр авторизации
Похожие вопросы