Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Peer review web-app-from-scratch-2021 week 2 #5

Open
ofahchouch-gh opened this issue Feb 12, 2021 · 1 comment
Open

Peer review web-app-from-scratch-2021 week 2 #5

ofahchouch-gh opened this issue Feb 12, 2021 · 1 comment

Comments

@ofahchouch-gh
Copy link

Over de readme

  • Er mist nog een beschrijving van wat voor applicatie je aan het maken bent.
  • Er mist nog een beschrijving van welke API endpoints je gebruikt.

Over het actor en interaction diagram

  • Er mist nog een router zie ik, maar daar ben je vast nog mee aan de slag.

  • Je gebruikt 2x renderData? De eerste is renderData.js en de tweede heet ook renderData()? Is het niet handig om de eerste bijvoorbeeld ApiHandler te noemen?

  • Verder komen boven de attributen en onder de methods wil je een beetje de UML standaard aanhouden.

  • Het interaction diagram lijkt op een snelle kopie en paste van het actor diagram met wat pijltjes erbij. Je hoeft niet alle attributes en methods van een klasse erbij te zetten. Het zou ook beter zijn om de daadwerkelijke functies die je in je klassen hebt te gebruiken als beschrijving, of in iedere geval meer die richting op.

Over de code

  • Goed dat je met modules probeert te werken.
  • Je main kan wat korter, die wil je zo overzichtelijk mogelijk houden en niet hele functies in schrijven.
  • Probeer wat meer met abstractielevels te werken. Dat betekend dat je functies maakt voor logica die in stukken gehakt kan worden om het zo overzichtelijker te houden. Maar zo te zien ben je hier stap voor stap mee bezig.
  • Proeer wat meer naamgeving te gebruiken. Je fetchData omschrijft niet welke data er wordt gefetched en waarvoor dit wordt gebruikt. Zo is het lastiger lezen voor een andere programmeur. Dit gebeurd ook op andere plekken.
This was referenced Feb 19, 2021
@HappyPantss
Copy link
Owner

Hey Oussama, bedankt voor je feedback. Sorry voor de late reactie, maar hier ga ik zo snel mogelijk aan werken!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants