veerleprins / functional-programming

Functional programming to create a data visualization with D3 with an API from the RDW datasets. This is part of the @cmda-tt.
MIT License
0 stars 0 forks source link

Peer review Veerle Prins #3

Open joordy opened 3 years ago

joordy commented 3 years ago

Application of subject matter

Repo & Readme

πŸ‘ Valid .gitignore, license and readme. No node-modules or some other trash. πŸ‘ Description is clear, maybe an idea to add some tags?

❗In the readme I miss some sketches of the datavisualization you want to create. ❗Be aware that there is a .json in your .gitignore. If you use something like server, bundler or other additionary stuff, your package.json will not be submitted to github, so write specific which file can't be uploaded. ❗Try to add more comments to your code, this will make it easier for an outsider to read.

Interesting functional pattern

https://github.com/veerleprins/functional-programming/blob/f708a8bce423b1b75aceba2b600c831d70cc1c8f/my-local-server/js/script.js#L12

πŸ‘ It's really nice to see how complex you make an function to replace the RGB. Good to know that it's usable for different values!

❗The only thing I wonder is why you don't call these functions in one specific function. This ensures that in your fetch function, it is easier to read when you start using other functions. Even if you use modules, it would be nice to process everything there. Because of this you only have to use 1 function in your script to call everything.

Understanding

πŸ‘ In the few comments you have, you describe where the code comes from. Even when we are in a call for consultation, you can tell exactly which part of the code does what. πŸ‘ Debrief is clear, maybe an idea to tell which data the Volkskrant offers you? ❗Try to describe what the code does in your wiki. How did you made the code, and what went wrong?

Quality

πŸ‘ The code you write looks clean and well build up. πŸ‘ Most important part of the code is on top. πŸ‘ Writes reusable functions ❗Be aware, when you write ES5 and ES6 mixed together, you can get errors. It's not a bad practice to write normal functions. But on many points an arrow function is easier and clean to read. ❗Try to call everything inside one function to clean up the eye colors. It's easier to read for others.

Process

πŸ‘ You describe what you did on the days of the week. Easy for you to read back! ❗Try to formulate it in a short and powerful way. If something goes right or wrong, it is best to explain this on the page where you describe the code in your wiki.

veerleprins commented 3 years ago

Thanks @joordy for this feedback on my repo! I will definitely be using your feedback to improve my readme, code and wiki πŸ˜„