Jelmerovereem / functional-programming

MIT License
0 stars 0 forks source link

Peer feedback 29-10-2020 #3

Open SamSlotemaker opened 3 years ago

SamSlotemaker commented 3 years ago

Repo + readme

Gitignore aanwezig, misschien handig om sowieso alle node_modules te ignoren ipv de specifieke paths?

Repo description uitgebreid en zeer duidelijk, ingegaan op het concept, gebruikte/te gebruiken data kan misschien iets duidelijker.

Credits nog niet ingevuld, maar het tabje is aanwezig dus dit komt vast nog :)

Wiki

Debrief of uitleg van opdracht mist in wiki, deze is wel aanwezig in de readMe. Data cleaning progress wordt goed stap voor stap beschreven, functional patterns worden aangetoond. Achteraf lijken er geen verbeteringen zijn aangebracht aan de code, in ieder geval is dit niet aangegeven.

Code

Code van de oogkleuren wordt niet perse opgesplists in functies. Je zou de code kleiner en leesbaarder kunnen maken als je dingen als de if-else: https://github.com/Jelmerovereem/functional-programming/blob/88dfae5ea0f40324f04b98e780386af9a6ac886c/data-opschonen/static/script.js#L47 zou opsplitsen in functions, die je onder aan de code kunt declareren. Hierdoor blijft het aanroepen van de code een stuk overzichtelijker. Ook had ik graag iets duidelijkere comments gezien, het is mij niet volledig duidelijk wat het verschil is tussen de eerste 2 forEach loops, waarbij beide ook weer veel dubbele code gebruiken. Dit had voorkomen kunnen worden met de functions (herbruikbare code)

Je zou denk ik ook makkelijk een .map kunnen gebruiken om een array te maken met alleen oogkleuren, in plaats van steeds student.oogKleur.

Qua complexiteit denk ik dat je weet wat je aan het doen bent en ik weet ook vrij zeker dat je kunt uitleggen hoe je code werkt. Waar je denk ik vooral nog verbetering in kunt aanbrengen is dat je code niet perse enorm uitbreidbaar is door het weinig gebruiken van functies. Stel je wil nu ook favoriete kleuren toevoegen, dan gaat dit niet met de code die je nu geschreven hebt.

Jelmerovereem commented 3 years ago