PacoZG / janestotalwellness

Website development for fitness and health advice and professional support.
https://janestotalwellness.com
1 stars 0 forks source link

Full Stack project review #10

Open Jakousa opened 3 years ago

Jakousa commented 3 years ago

Full Stack project review

Here's a short review of your Full Stack course project. The comments in the review are suggestions that you can use in this project or in your future projects. None of the suggestions are required to get a passing grade.

User experience

What did I do??

Logged in using given credentials.

Navigated around

Added a note to Sylvester Stallone

Experience

Sweet login & language navigation.

Many pages with spinners. The spinners make the page look like there is something loading even though nothing is happening. I'd remove the unfinished pages and navigation entirely.

Code

Code is organized pretty well. I'm able to find all parts of the code where I'm expecting to find them. However, I found a number of things for you to improve, I'll list all my findings next.

2/3 of your frontend code is copy paste. Use a internationalization library like formatjs https://formatjs.io/docs/getting-started/installation/, i18next https://www.i18next.com/ or equivalent to get rid of multiples of the same component.

Remove all useless comments, https://github.com/PacoZG/My-project/blob/2520ca20666608ea87570c61ade76a51fd447147/src/components/Spanish/ClientEsp.js#L19-L20. Git is a better version control system than comments.

Use guard clause and return to avoid nested conditionals https://refactoring.com/catalog/replaceNestedConditionalWithGuardClauses.html

https://github.com/PacoZG/My-project/blob/2520ca20666608ea87570c61ade76a51fd447147/src/components/Spanish/SignUpFormEsp.js#L82-L123

vs

...
if (!country) {
  return dispatch(setNotification({
    message: 'Selecciona un país.',
    title: 'Campos sin llenar',
    show: true
  }))
}
if (!checkBox.checked) {
  dispatch(setNotification({
    message: 'Debes aceptar los términos y condiciones para poder registrarte',
    title: 'Selecciona la casilla',
    show: true
  }))
}

const createdUser = await userService.createUser(newUser)

I think it makes the code much easier to read (if no country then tell user no country, if not checked tell user to check). I'm also not a big fan of else.

Use guard clause to clean ternaries out of your return clause:

https://github.com/PacoZG/My-project/blob/2520ca20666608ea87570c61ade76a51fd447147/src/components/Spanish/ModalEsp.js#L15

vs

if (!messageData.show) return <div></div>

return (
  <div>
    ...
  </div>
)

Guard clause can be utilized in the backend as well: https://github.com/PacoZG/My-project/blob/2520ca20666608ea87570c61ade76a51fd447147/controllers/users.js#L54

const user = await User.findById(request.params.id)
if (!user) return response.status(400).end()

response.json(user.toJSON())

Last but not least, split your components into multiple smaller components. This file is almost 600 rows! https://github.com/PacoZG/My-project/blob/master/src/components/Finnish/EditFormFin.js It could most likely be split into at least a few smaller ones.

Consider using prettier https://prettier.io/ and eslint https://eslint.org/ to help yourself find issues in your code quality. At the moment your project probably would light up like a christmas tree, but if you do the work to clean everything I bet it would look sharp!

Conclusion

The application doesn't seem to have many working features, but what it has works great! And although the code as a lot of copy paste and has long functions there doesn't seem to be any fundamental problems with it. I hope you spend time to improve the code maintainability, quality and readability before continuing. I personally like the eslint airbnb config, it's pretty robust and should help you find issues that your code has. But before doing that use i18n libraries to fix the copy paste problem!

I hope you keep at it to create a portfolio-ready project!

PacoZG commented 3 years ago

Thanks for all the advices, I managed to do pretty much everything you adviced me to do, it all make sense and I'm happy for the results. I have some ideas that I need to work on.

I'll get back to you so hopefully you can get another look to the improvements.

BR

Paco