Commit 19f8c362 authored by George Adrian Stoica's avatar George Adrian Stoica
Browse files

Add slides for lecture 19

parent cb292b82
Pipeline #90524 passed with stage
in 1 minute and 14 seconds
= Common issues
:customcss: slides.css
:icons: font
:includedir: revealjs/includes/
:LECTURE_TOPIC: Common issues
:LECTURE_NO: 19th Lecture
include::{includedir}header.adoc[]
[.smaller-80][.center-paragraph]
IT1901 Fall 2020 - {LECTURE_NO}
[.smaller-80]
== Overview
- Plans vs Implementation
- Programming bad practices
- Testing
- Code quality
- Issues
- Merge requests
- Other matters
== Plans vs Implementation
- Litt vel stor forskjell mellom ambisjoner som beskrevet i planer,
skjermbildeskisser og faktisk innlevering.
- Mange virker ikke som et forsøk på å realisere det de har beskrevet.
== Programming bad practices
- En del kjernekode er rene data-klasser uten noen særlig logikk, heller ikke validering.
- Innkapsling brytes ved at interne lister returneres direkte
- Typing fields to specific implementations rather than an interface (ArrayList<> vs List<>)
[.notes]
--
* Innkapsling brytes ved at interne lister returneres direkte. Når slike lister returneres, er det et heller ikke mye vits i å implementere Iterable, fordi den har til hensikt å støtte iterasjon uten tilgang til underliggende liste.
--
[.smaller-80]
== Testing
- ikke test triviell kode; det er viktigere å teste metoder med logikk
- Det er ikke vits i a test toString()
- En del bruker try/catch og fail() galt
[.notes]
--
* En del tester av triviell kode, som gettere og settere som kunne vært generert, er helt unødvendige. Det er viktigere å teste metoder med logikk , f.eks. filtrering, oppdatering av data osv.
* Det er ikke vits i a test toString() når det ikke regnes som api for andre klasser.
* En del bruker try/catch og fail() galt, det er f.eks. ikke vits i å ha try { ... } catch (Exception e) { fail(); }, for testformål er det bedre å bare la koden kræsje.
--
[.smaller-80]
== Code quality
- Mange tar ikke hensyn til checkstyle-anmerkninger, og en del har ikke engang koblet det inn.
- Some will implement something just too fool Spotbugs
```java
// We need to implement this to make spotbugs shut up.
// Technically, we are supposed to make equals and compareTo
// agree, but we specifically want events to be sorted
// chronologically. The right way is probably to implement
// a comparator instead of overriding compareTo, and sorting
// with that.
@Override
@Deprecated
public boolean equals(Object o) {
// We can check quilcky by comparing pointers
if (this == o) {
return true;
}
return false;
}
```
== Issues
- Many create issues that lack the details and/or discussion (e.g. just the title)
- Some will not connect issues to assignee and / or milestones
- Not using these tools makes it harder to collaborate
- There is no point in using issues just to show that we use them
== Merge requests
- some groups use merge requests regularly (good)
- but they almost never comment and discuss code to improve it (bad)
[.smaller-80]
== Other matters
- For mange er opptatt av registrering/innlogging
- Oppdeling i flere kontrollere kan være vanskelig å gjøre ryddig
- Ikke bruk toString() til visning i UI-et
- Many add to the repository files that should be ignored
[.notes]
--
* For mange er opptatt av registrering/innlogging, men det kan ikke sies å være viktig å få feedback på fra brukere, selv om det selvagt vil være nødvendig.
* Oppdeling i flere kontrollere er ofte riktig, men kan være vanskelig å gjøre ryddig, f.eks. navigering mellom ulike Scene.
* Ikke bruk toString() til visning i UI-et, selv om det er en billig måte å f.eks. vise elementer i lister.
* Many add to the repository files that should be ignored (settings from IDE, files generated by OS, build folders etc) - use .gitignore
--
[background-color = "#124990"]
[color = "#fff6d5"]
== Summary
include::{includedir}footer.adoc[]
\ No newline at end of file
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment