Простые флажки с реагировать


Мне дали следующие упражнения как часть процесса собеседования. Я представил мой код и получил обратную связь, что мое решение - "закрыть", но не "правильный". Мой код выполняет просто отлично и показывает нужные результаты. Может кто-то пожалуйста, скажите мне, что сделал бы мой код более "правильный"? или - неужели это все требования? (Я понимаю, что конструктор уже не нужен, Но сайт CodePen жаловался без него).

Требования:

Используя data переменная реализовать небольшую форму чекбоксами

[x] Sonia
[ ] Maria
[ ] John
[x] Michael

На каждый форме изменить на любой флажок печатать его текущее состояние, которое должно выглядеть data.

const data = {
  sonia: true,
  maria: false,
  john: false,
  michael: true
};

Мое решение:

class App extends React.Component{

    constructor() {
        super();

        this.state = {
            data: {
                Sonia: false,
                Maria: false,
                John: false,
                Michael: false
            }
        };

        this.handleClick = this.handleClick.bind(this);
    }

    handleClick(event) {

        let data = this.state.data;

        const target = event.target;
        const value = target.type === 'checkbox' ? target.checked : target.value;
        const name = target.name;

        data = this.state.data;

        for (const i in data) {
            if (i === name) {
                data[i] = value
            }
        }
        this.setState( {data:data});
    }

    render() {
        return (
            <div>
                <form>
                  {Object.keys(this.state.data).map((key, index) =>
                        <div key={index}>
                            <input
                                name={key}
                                id={index}
                                type="checkbox"
                                checked={this.state.data[key].value}
                                onChange={(event) => this.handleClick(event)} />

                            <span className="padding">
                                <label htmlFor={index} key={index}>
                                    {key}
                                </label>
                            </span>
                        </div>
                    )}
                </form>
                <div className="center">
                    <h2>Current State: </h2>
                    <ul>
                        {Object.keys(this.state.data).map((key, index) =>
                            <li key={index}>{ `${key}:  ${this.state.data[key]}`}</li>
                        )}
                    </ul>
                </div>
            </div>
        );
    }
}
ReactDOM.render(<App />, document.getElementById('app'));


2368
7
задан 30 января 2018 в 09:01 Источник Поделиться
Комментарии
2 ответа

Главный Вопрос


Может кто-то пожалуйста, скажите мне, что сделал бы мой код более "правильный"?

Трудно точно знать, что сделал бы мой код более "правильным" , но это выглядит как способ handleClick является более сложным, чем он должен быть. В частности, for петли, которые не нужны. Возможно, вы пытаетесь защититься от случаев, когда name не на самом деле собственность на data. Это может быть достигнуто с помощью объекта.метод hasOwnProperty() - например, for петли можно заменить этот блок кода:

if (data.hasOwnProperty(name)) {
data[name] = value;
}

Или, возможно, даже проще будет использовать in оператора. Для объяснения различий, см. Этот ответ на так.

if (name in data) {
data[name] = value;
}

Бесполезные уступки data в handleClick способ

В начале handleClick способ, я вижу эту строку:


let data = this.state.data;

И потом, после хранения констант для целевого события и его название, есть эта линия:


data = this.state.data;

Эта линия, кажется, не служат никакой цели.

Ввод тип галку?

Существуют ли другие типы материалов, кроме галочки? Если нет, то следующую строку в handleClick() кажется слишком сложным:


   const value = target.type === 'checkbox' ? target.checked : target.value;

Отсутствуют значения атрибутов, устанавливается на любые элементы.

Обязательный метод в обработчик событий

В handleClick метод вызывается из функции стрелкой в onChange, после чего обработчик атрибута:


onChange={(event) => this.handleClick(event)}

В соответствии с ReactJS документации:


Проблема с этот синтаксис другой обратного вызова создается каждый раз, когда [компонент] оказывает. В большинстве случаев, это хорошо. Однако, если этого обратного вызова передается в качестве опоры для нижней компоненты, эти компоненты могут сделать дополнительную повторного рендеринга. Как правило, мы рекомендуем обязательный в конструкторе или с помощью синтаксиса поля класса, чтобы избежать такого рода проблемы с производительностью.1

Там нет необходимости, чтобы добавить дополнительную анонимную функцию, чтобы просто вызывать этот метод. Привязать метод непосредственно, как показано ниже:

onChange={this.handleClick}

Проверено атрибут

В проверено атрибут каждого флажка сигнала в соответствии со следующим:


checked={this.state.data[key].value}

Но нет необходимости использовать .value - она может быть упрощена, как показано ниже:

checked={this.state.data[key]}


1https://reactjs.org/docs/handling-events.html

2
ответ дан 1 февраля 2018 в 05:02 Источник Поделиться

Я думаю, что все Сэм Onela сказала, это прекрасно. Я собираюсь добавить еще несколько вещей.


Не изменяйте вашего государства напрямую

В общем, это считается плохой практикой, чтобы напрямую изменить состояние объекта. Согласно среагировать документы:


Никогда не мутируют этого.государство напрямую, а вызывая setState() впоследствии может заменить мутации, которые вы сделали. Относиться к этому.государство, как будто это были незыблемы.

Вместо этого вы должны либо


  • Пройти setState объект только что вы хотите изменить (я выбрал этот)

  • Клон вашего государства, изменить клона, затем передать его setState

Прекратить использование index подобное

Используя index в ваши петли вводит еще одну переменную вы не нужны, и на самом деле делает вещи немного менее правильно.

Не использовать index для key проп

В key проп должен быть уникальный ключ, идентифицирующий этот элемент.
Самые уникальные вещи мы имеем о каждом из этих флажков-это имя, которое идет с ними.
Если мы перестроили этот список, мы хотели бы имена, чтобы остаться проверил, не те же индексы в списке.

Не использовать index как id

По крайней мере, не напрямую.
Вы хотите, чтобы ваши идентификаторы должны быть уникальными на странице, используя коды 1, 2, 3и т. д. кажется, не лучший способ быть уникальным. Было бы лучше использовать что-то вроде ${checkbox}-${key} таким образом мы получаем "checkbox-Sonia", "checkbox-Maria"и т. д.

Он также используется, чтобы быть так, что коды необходимо начать с буквы и не числа, но я считаю, что это не так.

Вам не нужна дополнительная data слой в вашем государстве

Это только добавляет ненужные трудности преодолены. Просто поставить флажок значения права в своем государстве, и только перемещать их, когда вам нужно сделать более сложные вещи с помощью вашего государства.

this.state = {
Sonia: false,
Maria: false,
John: false,
Michael: false
};

handleClick может быть путь проще

Одна линия на самом деле:

handleClick(key) {
this.setState({ [key]: !this.state[key] });
}

Черт, она настолько мала, что я бы наверное даже поставил его в линию, но разные люди могут не согласиться на это.

Пересмотренный Вариант

Вот что мой пересмотренный код будет выглядеть так:

class App extends React.Component {
constructor(props) {
super(props);
// I guess you don't have props, so it doesn't really matter,
// but you need to pass them to `super()` if you want them
// to be available on `this.props`

// Doesn't need the extra `data` layer
this.state = {
Sonia: false,
Maria: false,
John: false,
Michael: false
};
}

// handleClick was inlined

render() {
return (
<div>
<form>
{Object.keys(this.state.data).map((key) => (
<div key={key}>
<input
name={key}
id={`checkbox-${key}`}
type="checkbox"
checked={this.state.data[key]}
onChange={() => this.setState({ [key]: !this.state[key] })}
/>

<span className="padding">
{/* Don't need a `key` prop here */}
<label htmlFor={`checkbox-${key}`}>
{key}
</label>
</span>
</div>
))}
</form>
<div className="center">
<h2>Current State: </h2>
<ul>
{Object.keys(this.state).map(key => (
<li key={key}>{`${key}: ${this.state.data[key]}`}</li>
))}
</ul>
</div>
</div>
);
}
}

Обратите внимание, что это по-прежнему использовать функцию стрелки и не связанный метод.
Я думаю, что это позволяет коду быть гораздо более читабельным, и возможно, потерь не поняли, пока у вас есть компоненты, которые реализуют PureComponent глубже в цепочке вызовов, получающих эту функцию (в настоящее время он идет прямо к <input>.

7
ответ дан 2 февраля 2018 в 05:02 Источник Поделиться