diff --git a/lectures/revealjs/19-common-issues.adoc b/lectures/revealjs/19-common-issues.adoc new file mode 100644 index 0000000000000000000000000000000000000000..88072b3f69a546a243a9baf555c0f015627a2f60 --- /dev/null +++ b/lectures/revealjs/19-common-issues.adoc @@ -0,0 +1,112 @@ += 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