Code review

[mikeyhogarth]

Did a quick code review, the advice here please feel free to either take or ditch, i had 10 mins spare and thought I'd take a look. Some comments are in-line, but in addition;

The good

• This looks a fuck of a lot better than my first React code.
• Most of the code is absolutely fine, no major issues

The bad

• You're still choosing to use classes over functions (there is no reason to use classes since 16.8)
• You are teetering on the brink of using the reducer pattern, but you've rolled your own out of the Context API. The way you've done it here won't scale - with multiple contexts appearing and providing to different parts of the component tree, one day it'll become unwieldy.

Your State management solution

• State should contain state - don't piggy back functionality in there.

• You are spreading business logic (particularly, state-update logic) throughout your components rather than centralising it. If/when you check out using dispatch/reducers (whether through the built in hooks or through redux/flux/mobx) what you'll see is;

  • Components dispatch actions that specify intent
  • Reducers respond to these actions and update state
  • Components subscribe to the state and re-render whenever it changes (or, in redux, whenever stuff they're interested in changes)

It means that when you are wondering about the logic of your app, say, logging someone in/out, you don't need to worry about which component that happens in because you know it's just going to be in a single reducer somewhere.

With your current setup, consider this: Imagine you wanted another logout button somewhere, or you wanted a certain action to force a logout. As you have it now, you'd need to extract that logic somewhere and re-implement your context stuff in the new place.

Using reducer/dispatch, you just dispatch a logout action.

[OperationXen]

Thanks for the review mate, I'm going to go through it line by line, but it looks like I need to read up on redux to handle the process of shuffling around actions as I accept totally your argument around the additional logout functionality example.

This "state maze" seems very much like a common issue with react apps, at least as I've been designing this one so I'm glad there's an accepted solution! As with react I'm going to go read the docs first...