ChapterRouter - Warning: Each child in an array or iterator should have a unique "key" prop.

[energydev]

There is a "unique key prop" warning occurring when going into a set of questions. Below is a picture.

[energydev]

I've seen this issue before and can tackle this one.

[energydev]

Do we want to consider adding an ID field to the Questions objects? This would allow us to have a unique key that isn't the index. This would provide more flexibility according to this article. Using the index, which is the default, can cause issues if the ordering of the items is changed.

https://medium.com/@robinpokorny/index-as-a-key-is-an-anti-pattern-e0349aece318

const Ch1Questions = [ { question: "How many expressions are in the following statement: a = b * 2;", answers: [ { answer: "one", isCorrect: false }, { answer: "two", isCorrect: false }, { answer: "three", isCorrect: false }, { answer: "four", isCorrect: true } ] },

[austintackaberry]

Thanks for opening this issue! It looks like we have this issue in ChapterRouter and in Question.

For ChapterRouter, I think a good solution would be to make the key equal to the path, since the path is a unique identifier.

        {chapter.questions.map((question, index) => {
          return (
            <Route
              path={bookUrl + chapter.url + "/q" + (index + 1)}
              render={() => (
                <Question
                  baseUrl={bookUrl + chapter.url}
                  index={index + 1}
                  question={question}
                  numberOfQuestions={chapter.questions.length}
                />
              )}
            />
          );
        })}

As for Question, I think you are correct in that there is no good solution without having to add ids to the data object. Having said that, we could just make the ids 0, 1, 2, and 3 (incorporating the change from https://github.com/austintackaberry/ydkjs-exercises/issues/21). Per the React docs, the ids do not need to be globally unique, only within the context of the array. So maybe something like this:

{
  question: "How many expressions are in the following statement: a = b * 2;",
  ordered: true,
  correctAnswer: {
    answer: "four",
    id: 3
  },
  incorrectAnswers: [
    {
      answer: "one",
      id: 0
    },
    {
      answer: "two",
      id: 1
    },
    {
      answer: "three",
      id: 2
    }
  ]
}

The reason I added an id to the correct answer here is because we will probably want to merge the correct answer with the incorrect answers in a helper function that we call in Question.js, so that we can just array.map() in the render method. If we don't assign it an id here, then we will need to assign it an id in Question.js.

Here's a potentially crazy idea. So I was thinking that most of the answers we would want the order to be randomized. But for the question above, it would be odd to show the answers in random order. So what if we added a key ordered that if true, means that the answers should be displayed in the order of their id? If ordered is false, then the answers should be displayed in a random order. Thoughts?

[energydev]

Lots of great ideas! I'll take a look at these ideas and the ones from Explanation and Force Single Answer and get back with you.

[rosaxxny]

Would it make a difference if you grouped all the answers together and then checked if the selected answer was the correct answer?

  question: "How many expressions are in the following statement: a = b * 2; ?",
  ordered: true,
  answers: [
    {answer: "one", id: 0},
    {answer: "two", id: 1},
    {answer: "three", id: 2},
    {answer: "four", id: 3}
  ],
  correct: "four"
  }

[austintackaberry]

Ooh I like that idea. But I think we should check that it is the correct answer via its id instead of its value:

  question: "How many expressions are in the following statement: a = b * 2; ?",
  ordered: true,
  answers: [
    {answer: "one", id: 0},
    {answer: "two", id: 1},
    {answer: "three", id: 2},
    {answer: "four", id: 3}
  ],
  correctAnswer: {id:3}
  }

[energydev]

I also like the idea of grouping all the answers into one array and then referencing the correct answer by id. I'd, also like to propose renaming ordered to something like "orderById". This provides more clarity as to the preferred ordering, as ordered could refer to something like the answers are already ordered.

I'd also like to propose adding an id to the question itself. This would allow us to use it as the key to resolve the warning instead of the longer url.

id: 0 question: "How many expressions are in the following statement: a = b * 2; ?", orderById: true, answers: [ {answer: "one", id: 0}, {answer: "two", id: 1}, {answer: "three", id: 2}, {answer: "four", id: 3} ], correctAnswer: {id:3} }

[energydev]

@austintackaberry if you, by chance, have a format you're excited about by tomorrow/Thursday morning, I've got time to knock out this along with issues #14 and #21.

[rosaxxny]

Has there been a decision on the format? You could also add a key for the explanation.

  question: "How many expressions are in the following statement: a = b * 2; ?",
  ordered: true,
  answers: [
    {answer: "one", id: 0},
    {answer: "two", id: 1},
    {answer: "three", id: 2},
    {answer: "four", id: 3}
  ],
  correctAnswer: 3,
  explanation: "explain why other answers are wrong and why this answer is the best answer"
  }

[austintackaberry]

Ok, I think the schema below is good. It fixes a lot of the issues with the current schema while still being easy to understand. Thanks for the discussion!

{
  question: "How many expressions are in the following statement: a = b * 2; ?",
  orderedById: true,
  answers: [
    {text: "one", id: 0},
    {text: "two", id: 1},
    {text: "three", id: 2},
    {text: "four", id: 3}
  ],
  correctAnswerId: 3,
  explanation: "explain why other answers are wrong and why this answer is the best answer"
}

[nikrb]

@energydev do you still have this in hand?

[energydev]

@nikrb @austintackaberry I can make these changes today.