2
\$\begingroup\$

https://codesandbox.io/s/react-quiz-kb1xb

I was wondering what could be improved upon it. I am looking at how it's using Redux and how it's also using hooks and I don't really see anything wrong with it, but I am wondering if it could be improved upon.

import React, { Component } from 'react';
import { connect } from 'react-redux';



class ResultTile extends Component {


    isCorrect(options, answers, index) {
        let i
        options.map((value, index1) => {
            if (value.isAnswer === true)
                i = index1
            return null
        })
        if (options[i].name === answers[index]) return true
        else return false
    }

    getAnswer(options) {
        let answer
        options.map((val, index) => {
            if (val.isAnswer === true) {
                answer = options[index].name
            }
            return null
        })

        return answer
    }


    render() {
        let key = 0

        return (
            this.props.questions.map((question, index) => {
                return (
                    <div
                        key={question.id - 1000}
                        className={this.isCorrect(question.options, this.props.answers, index) ? "alert alert-success mx-auto px-4 py-1 w-100 " : "alert alert-danger mx-auto px-4 py-1  w-100"}
                        role="alert">
                        <p className="font-weight-bold" > <strong> Q{question.id - 1000}.</strong> {question.name} </p>
                        <div className="container">
                            <div className="row">
                                <form className=" ">
                                    {question.options.map((option) => {
                                        return <div
                                            key={key++}
                                            className="form-check">
                                            <input
                                                className="form-check-input key"
                                                type="radio"
                                                checked={option.name === this.props.answers[index]}
                                                disabled />
                                            <label
                                                className="form-check-label">
                                                {option.name}
                                            </label>
                                        </div>
                                    })
                                    }
                                </form>
                            </div>
                        </div>

                        <div className="alert alert-dark p-1 mt-3 mb-2 w-100" role="alert">
                            Right answer is : <strong>{this.getAnswer(question.options)}</strong>
                        </div>

                    </div>
                )
            }
            )
        );
    }
}

const mapStateToProps = state => {
    return {
        answers: state.answers,
        questions: state.questions

    }
}
const mapDispatchToProps = dispatch => {
    return {
        updateBonus: () => dispatch({ type: "UPDATE_BONUS", value: 100 }),
    }
}


export default connect(mapStateToProps, mapDispatchToProps)(ResultTile);

You can check the whole app, or just this component in particular, because it's one of the bigger components.

\$\endgroup\$
2
\$\begingroup\$

There are many ways that you can use newer syntaxes to make this cleaner:

  • Function component instead of class component.
  • Replace connect with react-redux hooks.
  • Create your actions with redux-toolkit.
  • Write better loops with specific array methods.

For that last point about array methods, here's a good example:

getAnswer(options) {
    let answer
    options.map((val, index) => {
        if (val.isAnswer === true) {
            answer = options[index].name
        }
        return null
    })

    return answer
}

We could rewrite that with .find() to find the correct answer. Your current use of .map() doesn't make any sense because you aren't using the mapped array. You are using it like a forEach(). But find() is more appropriate to this situation.

getAnswer(options) {
    // Find the first option with a true value of `isAnswer`.
    const correctOption = options.find(option => option.isAnswer);
    // Return its name
    return correctOption?.name;
}

Which could easily be a one-liner:

const getAnswer = (options) => options.find(option => option.isAnswer)?.name;

But I would kill both that function and your overly-complex isCorrect function and evaluate them inside of your loop:

{questions.map((question, index) => {
    const selectedAnswer = answers[index];
    const correctAnswer = question.options.find(option => option.isAnswer)?.name;
    const isCorrect = selectedAnswer === correctAnswer;

You have some other functions that aren't entirely necessary, for example:

isAnswered = (index, answers) => {
   if (answers[index]) {
       return true
   }
   return false
}

This is the same as simply checking the boolean value of answers[index] with !!answers[index].

You could simplify it to a one-liner:

isAnswered = (index, answers) => !!answers[index]

But it's just as easy to write the !!answers[index] directly instead of calling isAnswered(index, answer).


Here's how you can rewrite your reducer with Redux Toolkit:

import questions from "./questions";
import { createSlice } from "@reduxjs/toolkit";

const initialState = {
  questions: questions,

  counter: 0,

  answers: []
};

const slice = createSlice({
  name: "quiz",
  initialState,
  reducers: {
    jumpToFirst: (state) => {
      state.counter = 0;
    },
    jumpToLast: (state) => {
      state.counter = state.questions.length - 1;
    },
    jumpToQuestion: (state, action) => {
      state.counter = action.payload;
    },
    prev: (state) => {
      state.counter--;
    },
    next: (state) => {
      state.counter++;
    },
    updateAnswer: (state, action) => {
      const { index, answer } = action;
      state.answers[index] = answer;
    }
  }
});

export default slice.reducer;

export const {
  jumpToFirst,
  jumpToLast,
  jumpToQuestion,
  prev,
  next,
  updateAnswer
} = slice.actions;

We now have action creator functions so that you can dispatch prev() instead of {type: "PREV"}. This ensures that your type names are always correct.


But we can slim down that reducer a lot because you might be suffering from a bit of action-overload. Though there's nothing wrong with these actions, you don't need jumpToFirst, jumpToLast, prev, or next as these are all specific instances of jumpToQuestion. You really only need jumpToQuestion and updateAnswer.

Here's how the QuestionNavigation component might look with a single action type, using a createButton helper function to cut back on the repetitiveness of the four buttons. Now if you want to change the button styling it's just one place instead of four.

import React from "react";
import { useDispatch, useSelector } from "react-redux";
import { jumpToQuestion } from "../redux/reducer";

export default function QuestionNavigation() {
  const lastIndex = useSelector((state) => state.questions.length - 1);
  const currentIndex = useSelector((state) => state.counter);

  const dispatch = useDispatch();

  const isFirst = currentIndex === 0;
  const isLast = currentIndex === lastIndex;

  const createButton = (text, index, disabled) => {
    return (
      <button
        onClick={() => dispatch(jumpToQuestion(index))}
        className="btn btn-sm btn-primary m-2"
        disabled={disabled}
      >
        {text}
      </button>
    );
  };

  return (
    <div className="container mt-2">
      <hr />
      <div className="row justify-content-center">
        {createButton("First", 0, isFirst)}
        {createButton("Prev", currentIndex - 1, isFirst)}
        {createButton("Next", currentIndex + 1, isLast)}
        {createButton("Last", lastIndex, isLast)}
      </div>
      <hr />
    </div>
  );
}

Here's my revised version of the ResultTile:

import React from "react";
import { useSelector } from "react-redux";

export default function ResultTile() {
  const questions = useSelector((state) => state.questions);

  const answers = useSelector((state) => state.answers);

  return (
    <>
      {questions.map((question, index) => {
        const selectedAnswer = answers[index];
        const correctAnswer = question.options.find((option) => option.isAnswer)?.name;
        const isCorrect = selectedAnswer === correctAnswer;
        return (
          <div
            key={question.id}
            className={
              "alert mx-auto px-4 py-1 w-100 " +
              (isCorrect ? "alert-success" : "alert-danger")
            }
            role="alert"
          >
            <p className="font-weight-bold">
              <strong> Q{question.id - 1000}.</strong> {question.name}
            </p>
            <div className="container">
              <div className="row">
                <form className=" ">
                  {question.options.map((option) => (
                    <div key={option.name} className="form-check">
                      <input
                        className="form-check-input key"
                        type="radio"
                        checked={option.name === selectedAnswer}
                        disabled
                      />
                      <label className="form-check-label">{option.name}</label>
                    </div>
                  ))}
                </form>
              </div>
            </div>

            <div className="alert alert-dark p-1 mt-3 mb-2 w-100" role="alert">
              Right answer is: <strong>{correctAnswer}</strong>
            </div>
          </div>
        );
      })}
    </>
  );
}

Complete CodeSandbox

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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