Redditor review

[gonzofish]

Hey, I'm the guy from reddit who asked for the repo...I'll just put my comments here, cool?

[enfrte]

Sure, thanks.

[gonzofish]

My first comment is that you're using a lot of global state via Context. You can accomplish some of this with using local state passed down to the child components through props.

Why is this a good idea? Whenever you update the context all the components that use it are going to re-render, so that means even parts of your app that don't need to change may run a re-render.

A good rule of thumb is this: if something is going to be needed app-wide, make it global. Otherwise, let the component handle it.

The app would benefit from using a router (React Router)

Format the Data

The data that you get from MongoDB can be transformed when it gets to the UI. You know how the UI is going to use it:

1. You're going to list each module on the menu
2. You're going to use each module's questions when that module is selected.

So put the data in a format that's going to suit your needs. My opinion is you actually need (at least) two data structures:

1. modules: An array of module descriptions containing the title and _id, since that's all the menu is using.
2. questions: An object for module questions where the keys are the _id and the questions are the array of questions in that module.

I would all format the questions so that instead of being nativeLanguage and foreignLanguage, they would be en and fi, respectively. All these changes make data access quicker. If you had a massive amount of data, the considerations might be different, but this is a small amount of data to work with, so doing these transforms when the fetch completes is ultimately a time-save.

Also, it'll get rid of more global state.

Use a Router

This way, instead of needing to track if you're on the menu or a module, you can have the URL take care of it. You're URLs could be: URL	Purpose
finnish-practice/:language	Home page/menu for the give language
finnish-practice/:language/:id	Show the module for the given language with the specified id

Just these two URLs will remove 4 pieces of global state:

• languageSwitch & setLanguageSwitch: this is now handled
• activeModule & setActiveModule: this is now handled by the id parameter of the URL; instead of a true/false switch, you can just check if the language is "en" or "fi"

Using language and id

When the user selects a module you update it in the context as the activeModule (the ID of the module) and selectedPractice (the object for each module). Before I mentioned that transforming the data would remove global state. I was mainly referring to selectedPractice. Instead of having a selectedPractice, different components could use questions and modules to suit their needs.

The questionIndex

I would also move the tracking of the questionIndex to be controlled by ModulePractice by using useState and pass it down to the child components. questionIndex is being used by ModulePractice and ModulePractiveAnswerResult. You're already doing this but passing it from the context to ModulePractice and then to ModulePracticeAnswerArea and then to ModulePracticeAnswerResult. So there's not reason to have ModulePracticeAnswerResult use the context for questionIndex when the value is already coming in as the questionNumber prop.

So you can get rid of this line and change this useEffect to read:

useEffect(() => {
  setCheckAnswer(false); // answer state is reverted when question state is updated
}, [questionNumber]);

---

That's all for now, hope this helps!

[enfrte]

Thanks, that was a great review. I'll work on addressing these issues straight away.

[gonzofish]

No problem! I can take a look when you've updated, if you'd like

[enfrte]

That's very kind of you. I'll let you know here when I have something ready.

[enfrte]

Hi, Thanks again for the review. I've learned a lot from the needed changes. I made a new branch for these changes: https://github.com/enfrte/finnish-expression-practice/tree/review-edit/react-version

I've now formatted the data coming from MongoDB into 3 groups; modules, questions, and tutorials.

Question properties are now in the format of "en" and "fi".

I'm now using react-router for the views; module listings (menu), practice questions, and tutorials. I managed to put the module id in the url, but ran into problems when trying to also add the language. For this reason, language is still controlled in the context store.

questionIndex is now handled in Practice as a prop and passed down to child components as questionNumber.

I realise there might still be the need for further work on this, but your suggestions have really turned my app from a mess, to something which is much more succinct, easier to read and reason with.