Peer review

[sk222sw]

Hej, här kommer min peer review på din labb 3 :)

Tyvärr får jag problem med CORS så jag har inte lyckats testa hela applikationen. Om du använt cloud9 till både API:et och din klientapplikation så har det kanske funkat iallafall, så var det för mig. Det finns ett smidigt gem som heter rack-cors som löser detta ganska smidigt.

Ett tips kan vara att deploya både serverapplikationen och klientapplikationen, då vet du säkert att det finns en version som går att testköra.

• I din kod finns det en del DRY. URL:en till API:et och headers finns med i varje request, trots att du har dem som konstanter i app.js.
• Din HTTP-hantering är lite osmidig: nu använder du $http i dina service-klasser men även i SearchController och CRUDController. Det borde bara finns i service-klasserna.
• Du injectar även $http i nästan alla service-klasser och det blir en hel del upprepning här. Det smidigaste hade kanske varit att ha en http-service som dina andra services använder. Flödet hade då blivit t.ex. PubController -> PubService -> HttpService -> API. Fördelen då är att du endast behöver en funktion i hela applikationen som gör GET-requests, en för POST, en för PUT och en för DELETE, och det blir mycket enklare att underhålla och göra ändringar osv.
• Du injectar en del grejer som inte används, t.ex. $q i CRUDPubService eller filterFilter i PubEditController
• Det finns också en del "boilerplate-kod" kvar från när du skapade projektet, t.ex. js/services.js som bara tar onödig plats och gör koden lite jobbigare att sätta sig in i.
• När man kör applikationen har jag bara sett två småsaker nu: 1. kartan går att zooma/dra i om man kör mobilläge, men inte desktopläge, det kanske går att lösa smidigt med nån inställning? 2. Även om man inte skriver i nånting i inloggningsformuläret så skickas en request till servern.

I övrigt så tycker jag din kod ser bra ut. Den är tydlig och lättläst. Det är inte så mycket kommentarer, men det tycker jag inte behövs, då det mest är standard angular-grejer. Vyfilerna ser bra ut, men det hade kanske varit bättre att inte ha input-fält i p-taggar, utan i div-taggar? :)

Säg till om du fixar CORS, så kan jag kika lite mer på resten av applikationen om du vill :)

[Bejta]

Hej. Jag har redan installerat rack-cors gem, och körde lokalt med webstorm, och API på cloud 9. Angular applikation på porten 3000 och API på 8000.

• Det finns en del DRY, för att jag inte hunnit med ordentligt debugging. Den kod används inte (i vissa service, t.ex. CRUDController och PubDetailController).
• Samma gäller med injecting och boilerplate-kod. Om man följer commits ser man att jag har börjat på nytt en gång (node http-server funkade inte med deep linking, så jag vände mig till nodejs express) och mycket boilerplate och överflödig kod är kvar.
• En validering på inloggningsformuläret hade varit bra :-). Det ska jag fixa.