简单看板组件

Question

我正在寻找一些关于如何使我的代码更简洁和更简单的建议。或者说是另一种方式:更优雅、更地道。任何其他关于质量或任何事情的评论都将不胜感激。

class App extends Component {
      constructor(props){
        super(props);

        this.state = {
            colIds : ["winnie", "bob", "thomas", "george"], 
            cols : [
              {name:"Winnnie", headerColor: "#8E6E95", id : "winnie" },
              {name:"Bob",  headerColor: "#39A59C", id :"bob"},
              {name:"Thomas",  headerColor: "#344759", id: "thomas"},
              {name:"George",  headerColor: "#E8741E", id: "george"}
            ],
            cards : [
              {colid: "winnie", task: "buy eggs"},
              {colid: "bob", task: "clean basement"},
              {colid: "thomas", task: "pack"},
              {colid: "george", task: "pay bills"},
            ]
          }

          this.addCard = this.addCard.bind(this);
          this.moveCardRight = this.moveCardRight.bind(this);
          this.moveCardLeft = this.moveCardLeft.bind(this);
      }

      getColCards(colid){
          let cards = this.state.cards.filter( c => c.colid == colid);
          return cards;
      }

      addCard(colid){
        let cardTask = window.prompt("Whats the task you want to add");
        let currentCards = this.state.cards.slice();
        let newCard = {colid: colid, task: cardTask};
        currentCards.push(newCard);
        this.setState({cards: currentCards});

      }
      moveCardRight(card){
        let currentCardCol = card.colid;
        let nextIndex =  this.state.colIds.indexOf(currentCardCol) + 1;

        if(nextIndex > this.state.colIds.length - 1 ){
          return null;
        }
        let currentCards = this.state.cards.slice();
        for(let i = 0; i < currentCards.length; i++){
            if(currentCards[i].task === card.task){
               currentCards[i] = {
                 ...currentCards[i],
                 colid : this.state.colIds[nextIndex]
               }
            }
        }
        this.setState({cards: currentCards});
      }

      moveCardLeft(card){
        let currentCardCol = card.colid;
        let nextIndex =  this.state.colIds.indexOf(currentCardCol) - 1;

        if(nextIndex < 0 ){
          return null;
        }
        let currentCards = this.state.cards.slice();
        for(let i = 0; i < currentCards.length; i++){
            if(currentCards[i].task === card.task){
               currentCards[i] = {
                 ...currentCards[i],
                 colid : this.state.colIds[nextIndex]
               }
            }
        }
        this.setState({cards: currentCards});
      }
      render() {
        let cols =[  
                      {name:"Winnnie", headerColor: "#8E6E95"},
                      {name:"Bob",  headerColor: "#39A59C"},
                      {name:"Thomas",  headerColor: "#344759"},
                      {name:"George",  headerColor: "#E8741E"}
        ];
        let colCards = this.state.cols.map(c => {
        let cards = this.getColCards(c.id).map(c =>  {
            return <div>
                    <span onClick = {() => this.moveCardLeft(c)}> {"<   "} </span> 
                    {c.task}   
                    <span onClick = {() => this.moveCardRight(c)}> {"    >"} </span> 
                   </div> 
          })

          return  <CardCol  name={c.name}  headerColor={c.headerColor} addCard={this.addCard} id={c.id}>
                     {cards}
                  </CardCol>
        })
        return (
          <div className="App">
          {colCards}
          </div>
        );
      }
    }

Answer 1

您的实现很棒，工作也很好，但是有一些事情您可以更改。

消除重复代码

有两个很大的方法，moveCardLeft和moveCardRight，它们几乎做同样的事情。可以完全组合这些代码，也可以将匹配的代码分离到一个新的方法中，并由原始的两个方法引用。以我为例，我选择了前者：

moveCard(card, dir) {
  let nextIndex = this.state.colIds.indexOf(card.colid);

  if (dir === "left") nextIndex--;
  else if (dir === "right") nextIndex++;
  else return null;

  if (nextIndex < 0 || nextIndex > this.state.colIds.length - 1) {
    return null;
  }

  let currentCards = this.state.cards.slice();
  for (let i = 0; i < currentCards.length; i++) {
    if (currentCards[i].task === card.task) {
      currentCards[i] = {
        ...currentCards[i],
        colid: this.state.colIds[nextIndex]
      }
    }
  }
  this.setState({ cards: currentCards });
}

这样，您只需为方向(左或右)传递一个额外的参数。它消除了几乎总是有益的重复。

使用一致格式

这包括缩进、新行和一般间距。

当您在文章中写入它(可能这只是一个复制/粘贴错误)时，呈现方法中的map中的map没有正确地缩进，这在某种程度上无疑会使您感到困惑。下面是它应该是什么样子：

render() {
  let cols = [
    { name: "Winnnie", headerColor: "#8E6E95" },
    { name: "Bob", headerColor: "#39A59C" },
    { name: "Thomas", headerColor: "#344759" },
    { name: "George", headerColor: "#E8741E" }
  ];
  let colCards = this.state.cols.map(c => {
    let cards = this.getColCards(c.id).map(c => {
      return (
        <div>
          <span onClick={() => this.moveCard(c, "left")}>{"<   "}</span>
            {c.task}
          <span onClick={() => this.moveCard(c, "right")}>{"    >"}</span>
        </div>
      );
    });

    return (
      <CardCol name={c.name} headerColor={c.headerColor} addCard={this.addCard} id={c.id}>
        {cards}
      </CardCol>
    );
  });

  return (
    <div className="App">
      {colCards}
    </div>
  )
}

其他想法

• 默认情况下使用严格相等(===)，并且只有在绝对必要时才返回到松散/抽象相等(==)。
• 考虑一下，如果您摆脱了colIds，只使用cols来组织卡片，那么代码会更加简洁。另外，考虑将cards放在各自的cols中，而不是将它们分离到自己的对象中。我还没有在您的代码中尝试过这一点，因此它可能最终会变得不那么简洁，但是在编写任何东西时考虑不同的数据结构是很好的。