facn7 / ek-week9-sudoku Goto Github PK

Flicks at 3..2..1..

You currently have your api url hardcoded and publicly available on the repo. Maybe this doesn't matter for Sudoku API but it's good practice to use environment variables for API urls / tokens / secrets.

Use this guide for how to set environment variables with Create React App:
https://github.com/foundersandcoders/react-week/blob/master/project.md#api-secrets

What exactly is idx? I see you use it in a couple of places (eg: here and here). At one point you pass it in as a row prop to <SubBoard>.

These names might make sense to you as the original author, but imagine you are an outside developer coming to this project for the first time, or you're just returning to the project after months or years. You have to waste time and cognitive energy trying to work out what these things are! Make it easy to understand by using helpful variable and function names that describe what they are / what they do. Don't worry about longer names (as long as they're not ridiculous).

You are using let for declaring/assigning new variables, when you should use const.

Example

Remember, you only need to use let when you are going to reassign the value of that variable later on. If the value will not change after the first time you assigned it, then you should use const.

This is the same even if the variable is an array and you are pushing new values inside. You would still use const. Eg:

const arr = []
arr.push('new value')

Pls consider adding some simple validation

You're setting a key attribute on <tr> inside SubBoard.js (see here).

You don't need to do this, as you already set a key attribute on <SubBoard> inside SudokuTable.js (see here). They are essentially the same element.

Lots of impressive stuff!

• Interesting use of Sudoku API, you've had to do some complex logic and it all seems to be working 😄
• Very pleased to see you extracting larger logic functions into their own files in /utils 👍This stops your components getting massive
• Loving the count down at the start, and good to see the board is disabled until it finishes
• You've done well at extracting different parts into React Components, which was one of main lessons this week.

The main improvement I'd suggest is working on the maintainability and readability of your code. Try to always imagine how it would feel for an outside developer to look at, and work on extracting sections of your code into functions with clear names to describe what they are doing. Also give variables clear names to describe what they are. This is crucial when you start coding professionally, in a team or by yourself - coming back to code after a while is often almost the same as being a stranger 🕵

Also, if you want to develop this project further, think about writing tests for complex logic - you want to be sure that any future changes you make don't break anything!

Nice count down when the app boots up!

Why not tidy up your React.useEffect() in App.js by extracting code into functions to make it more readable? You'd end up with something like:

React.useEffect(() => {
    getBoard(url).then(data => {
      initialiseBoards(data, setiBoard, setBoard)
      startCountDown()
    });
  }, []);