Emilvdijk / Lightning

Calculate distance from a thunderstorm by measuring time between lightning and thunder. also calculate time until the thunderstorm gets to you through multiple measurements in a loop.
0 stars 0 forks source link

Lightning Feedback #1

Open gglerum opened 2 months ago

gglerum commented 2 months ago

Ik vind het goed om te zien dat je met een switch en de Scanner over weg kan. Ook de toevoeging van het vragen om de tijd tussen de vorige en huidige meting is iets waar ik zelf niet aan gedacht had. Ik weet niet of de berekeningen kloppen, ik krijg namelijk bij mijn derde meting negatieve waarden. Niet dat dit voor deze opdracht wat uitmaakt, de code zelf vind ik veel belangrijker.

De code ben ik goed doorlopen, en ik heb wel een aantal punten. Er zijn niet echt "fouten" te noemen. Eerder dingen die je nog niet kon weten en een aantal optimalisatie slagen.

https://github.com/Emilvdijk/Lightning/blob/f51e8dd3907777a1ea8c0641ffedae3103718e76/src/main/java/nl/emilvdijk/App.java#L13 Ik weet dat in het boek staat omschreven dat dit zo kan en mag. Er zijn echter code standaarden waar wij ons als programmeurs aan moeten voldoen die zeggen dat dit niet mag. Ik weet dat het extra regels code toevoegt. Maar graag elke declaratie van een variabel op zijn eigen regel.

https://github.com/Emilvdijk/Lightning/blob/f51e8dd3907777a1ea8c0641ffedae3103718e76/src/main/java/nl/emilvdijk/App.java#L21 Je gebruikt zowel Scanner als System.in.read. Is daar een reden voor?

https://github.com/Emilvdijk/Lightning/blob/f51e8dd3907777a1ea8c0641ffedae3103718e76/src/main/java/nl/emilvdijk/App.java#L21-L25

Waar is deze code voor? Het negeert een new line, dat snap ik. Maar waarom? want je assigned de input al aan units op de regel daar boven.

https://github.com/Emilvdijk/Lightning/blob/f51e8dd3907777a1ea8c0641ffedae3103718e76/src/main/java/nl/emilvdijk/App.java#L26 Let op het verschil tussen | en ||.

Als || word gebruikt kijkt Java eerst naar de linker expressie van de statement: in dit geval: units == 'F'; als deze expressie waar is kijkt Java niet meer naar de rechter expressie: units == 'M'.

Als | wordt gebruikt (zoals in deze snippet) dan kijkt Java eerst naar de linker expressie van de statement en als deze waar is kijkt Java daarna ook naar de rechter expressie van de statement.

In deze snippet maakt het verschil niet veel uit. Maar als je bijvoorbeeld later met methodes (zoals we in Java functies noemen) dan kun je dingen doen zoals het volgende:

while ( !password.isValid() & (attempts++ < MAX_ATTEMPTS) ) {
    // ask for password imput
}

De rechter expressie wordt altijd uitgevoerd ook al is de linker expressie waar. Dus voor elke poging wordt attempts opgehoogt.

Als je niet bewust bent van het verschil tussen de non-short-circuit | en de short-circuit || dan kan je code zich anders gaan gedragen dan je verwacht.

https://github.com/Emilvdijk/Lightning/blob/f51e8dd3907777a1ea8c0641ffedae3103718e76/src/main/java/nl/emilvdijk/App.java#L53 Deze loopt draait maar één keer. Je kunt dus eigenlijk de loop weghalen en de inhoud daarvan laten staan, en dan zal de code het zelfde blijven doen.

https://github.com/Emilvdijk/Lightning/blob/f51e8dd3907777a1ea8c0641ffedae3103718e76/src/main/java/nl/emilvdijk/App.java#L31-L50 Wat ik zelf altijd heel leuk vindt om te doen is om te kijken of ik mijn eigen, of de code van een ander simpeler kan maken. Dat doe ik door te kijken wat eigenlijk het verschil is tussen twee "cases". In deze snippet zit het verschil in de unit (feet of meters) en de "modifier" die je toepast om de afstand in feet goed te weergeven.

Al we van de "unit" uitzondering een variabel maken String unitText en een variabel voor de modifier toevoegen double unitModifier en deze allebei declareren wanneer de gebruiker een eenheid heeft gekozen, dan kunnen we de switch weglaten:

System.out.println("\nPlease enter the time between the lightning and the thunder in seconds:\n");
timeBetweenTAndL = myScanner.nextDouble();
distanceFromBolt = timeBetweenTAndL * 343 * unitModifier; //modifier toegevoegd
previousDistance = distanceFromBolt;

//output the measurement in the appropriate units
System.out.println("The distance between you and the lightning is:" + distanceFromBolt + " " + unitText);

Je maakt nu gebruik van twee while loops om de metingen uit te lezen. Kijk eens of het lukt om het verschil tussen deze twee loops te vinden en het met één loop op te lossen.

https://github.com/Emilvdijk/Lightning/blob/f51e8dd3907777a1ea8c0641ffedae3103718e76/src/main/java/nl/emilvdijk/App.java#L55

Ik denk dat dit een bewuste keuze is (er stond namelijk niets in de opdracht), maar ik benoem hem toch. Deze loop heeft geen conditie om uit de loop te breken. De applicatie zal altijd blijven draaien. Nu kan de gebruiker het venster gewoon sluiten en daarbij de applicatie. Helaas bestaan er ook digibeten die dit niet snappen.

Als je een echte applicatie voor iets gaat maken, vergeet dan niet de optie toe te voegen om de applicatie af te sluiten.

Emilvdijk commented 2 months ago

Je gebruikt zowel Scanner als System.in.read. Is daar een reden voor?

ik las ergens op het internet dat read beter is voor single Char inputs. maar ik heb nog weinig ervaring met beide dus ik dacht pak ze allebei en ik zie wel wat ik fijn vind.

Waar is deze code voor? Het negeert een new line, dat snap ik. Maar waarom? want je assigned de input al aan units op de regel daar boven.

read leest een voor een de characters in je input en met dit stukje gooit hij alle charachters in de input na de eerste in de prullenbak. dit heb ik uit het boek overgenomen.

Als je niet bewust bent van het verschil tussen de non-short-circuit | en de short-circuit || dan kan je code zich anders gaan gedragen dan je verwacht.

ik had de verschillen gelezen ja, ik hoop dat ik het nog weet als het wel uitmaakt

Deze loopt draait maar één keer. Je kunt dus eigenlijk de loop weghalen en de inhoud daarvan laten staan, en dan zal de code het zelfde blijven doen.

correct.

Al we van de "unit" uitzondering een variabel maken String unitText en een variabel voor de modifier toevoegen double unitModifier en deze allebei declareren wanneer de gebruiker een eenheid heeft gekozen, dan kunnen we de switch weglaten:

clever!

Je maakt nu gebruik van twee while loops om de metingen uit te lezen. Kijk eens of het lukt om het verschil tussen deze twee loops te vinden en het met één loop op te lossen.

ik weet niet of dit handig is, ik gebruik andere prints in de eerste en begin pas met de eta te berekenen vanaf de 2e meeting want dan pas kan het.

Als je een echte applicatie voor iets gaat maken, vergeet dan niet de optie toe te voegen om de applicatie af te sluiten.

zeker ik weet nog niet hoe ik dat zou doen in java tenzij ik na elke loop eerst vraag of je nog een meeting wil doen. ik dacht dat zit alleen maar in de weg. kan ik een scanner gooien die en een double en een string aanneemt?.

een ding waar ik tegenaan kwam is dat hij lokale regels pakt oftewel hij wilt een comma in plaats van een punt voor een double. in ieder geval bedankt voor de feedback!