Automatisk kodinspektion

En hel del potentiella problem i programkod kan upptäckas automatiskt, genom algoritmer som analyserar koden och upptäcker specifika mönster. IDEA har över 600 sådana "kodinspektioner", och vi har satt upp en profil (TDDD78-årtal-version) som är anpassad till kursen. Detta ger omedelbar återkoppling, leder till färre kompletteringar då kvaliteten höjs före inlämningen, och ger handledarna mer tid till icke-trivial vägledning.

Den inspektionsprofil (konfiguration) vi använder i kursen delar in varningar i flera typer. Dessa färger används vid ljus bakgrund:

• ERROR (röd): Felaktig kod, kan normalt inte kompileras

• SEVERE (orange): Allvarligt problem

• WARNING (gul): Generell varning

• COMPLEX (lila): Kan vara komplicerat / svårläst / svårförståeligt

• OPTIMIZE (blå): Ineffektivt, kan optimeras

• STYLE (grön): Kodstil, tydlighet och läsbarhet

• DOC (grön): Dokumentationsproblem

1. #inspektion Har du åtgärdat eller kommenterat alla inspektionsvarningar?

   Alla projekt ska analyseras i IDEA, via Analyze|Inspect Code, med inspektionsprofilen TDDD78-2020-v1. Du kan köra en annan miljö för utvecklingen, men koden ska ändå analyseras i IDEA före inlämning.

   Automatisk kodanalys kan leda till falska varningar: Om en mekanisk analys helt och hållet skulle undvika det skulle man också missa många väldigt värdefulla varningar. Därför krävs att du för varje varning:

   • Antingen fixar varningen så den försvinner,

   • eller kommenterar varningen i koden, nära varje enskild varning, och motiverar varför ni faktiskt har gjort rätt och varför man inte bör ändra koden just där. Givetvis kan man hänvisa till längre förklaringar på annan plats, men det måste finnas en hänvisande kommentar nära själva varningen.

   Är du osäker på om varningen är korrekt, varför den ges, eller hur den kan åtgärdas? Skriv inte bara att du inte vet, utan ta aktivt reda på vad som gäller!

   • Läs den inbyggda förklaringen (klicka "more", eller sätt markören på det färgmarkerade felet och tryck Ctrl-F1).

   • För vissa varningar är IDEAs beskrivningar inte så pedagogiska. Därför förklaras många varningar mer ingående på den här sidan. Sök efter delar av varningsmeddelandet!

   • Om detta inte räcker, fråga handledaren. Och meddela gärna examinatorn, så kan vi utöka sidan!

   • Om handledaren inte har svar, fråga examinatorn.

   Att motivera på ett korrekt sätt ingår i examinationen. Att inte motivera kvarvarande varningar, eller att enbart kommentera att "jag tycker inte det behövs" eller att "IDEA har fel", ger normalt komplettering!

Allmän kodstruktur och organisation

Läsbarhet, struktur och organisation är viktigt eftersom programmering ofta går ut på att utöka och ändra existerande kod. Tid som läggs ner på att göra kod lättläst och välstrukturerad, och att dokumentera, är alltså inte bortslösad utan sparas in med råge i senare utvecklingsfaser. Slutmålet är därför inte bara att skapa ett program som åstadkommer rätt saker, utan att skapa ett program som gör på rätt sätt.

Här ger vi några konkreta exempel på vad ni kan tänka på för att förbättra läsbarhet, struktur och organisation. Detta är långt ifrån uttömmande!

1. #klassansvarHar klasser tydliga ansvarsområden?

   Under en föreläsning gavs exempel på klasser som gör många olika saker och som är svåra att beskriva med en enda mening. Sådana klasser kanske egentligen ska vara flera.

2. #metodansvarHar metoder tydliga ansvarsområden, lagom omfattning?

   Om en metod gör flera separata saker (flyttar alla spelobjekt, kontrollerar kollisioner, applicerar powerups, uppdaterar spelets allmänna tillstånd) är det ofta bra att dela upp den i flera. Om en metod är väldigt lång gör den troligen flera separata saker. IDEA: Refactor / Extract / Method.

3. #paketAnvänder du paket för att strukturera?

   All kod måste ligga i ett paket. I nästan alla projekt är det bra att dela upp koden i flera paket, där klasser som hör ihop (t.ex. alla klasser för powerups, logikformler, ...) ligger i samma paket.

   Paketindelning sker normalt efter funktionella gränser. Man delar alltså inte upp koden efter språkbegrepp, t.ex. alla enum-typer i ett paket och alla exceptions i ett annat paket. Istället handlar det om att strukturera koden efter hur den "hör ihop" funktionalitetsmässigt när det gäller begrepp som hör till själva projektet.

   Class without package statement / Class '...' lacks a package statement – Varnar för klasser som inte alls ligger i paket (men uppdelningen i olika paket får man själv ta hand om!).

   FileWithoutPackage

   SinglePackage

4. #DÖDKODHar du tagit bort eventuell död (oanvänd) kod?

   Kod som inte testas eller demonstreras ska inte lämnas in utan tas bort ur projektet. Detta gäller speciellt om det handlar om många olika kodstycken (fält, metoder, klasser) som inte används just nu.

   Undantaget är om det finns större stycken sammanhängande kod som inte är färdig, men där ni ändå vill visa att ni har kommit en bit på vägen. Sådan kod måste vara väl avgränsad och du måste tydligt dokumentera vilka delar (hela klasser eller metoder) som inte används och varför. Det som inte används måste alltså vara väl ihopsamlat så att det lätt kan ignoreras av kodgranskare.

   Unused Declaration – med hjälp av denna inspektion kan IDEA till stor del analysera vilka delar av ett program som faktiskt används.

   Då måste man veta var man egentligen utgår ifrån, dvs. varifrån någon kan "starta" programmet, till exempel från vilken main()-metod som helst i programmet. Sådana startpunkter kallas "entry points". Dessa är alltså inte fel i sig själva utan talar bara om var IDEA börjar leta efter kod som kan användas.

   Efter detta visar inspektionen de fält, metoder och klasser som inte kan nås från någon entry point via metodanrop med mera. Om det syns något här som faktiskt används, beror det på att IDEA inte var medveten om någon entry point. Till exempel kan det hända att en enum-konstant enbart används indirekt via Enum.valueOf(String). Då står enum-konstanten aldrig i koden, och IDEA kan missa användningen. Tala isåfall om via en kommentar hur ni egentligen använder den kod som felaktigt har markerats som oanvänd.

   I övrigt är det alltså meningen att ni ska ta bort oanvänd kod före inlämning, eftersom den är otestad och odemonstrerad.

Kortfattad och koncis kod

Inlämnade projekt innehåller ofta kodsnuttar som upprepas, identiskt eller med små variationer. Upprepningarna kan ske inom en och samma metod, mellan metoder i samma klass, eller mellan olika klasser.

Detta är problematiskt eftersom man får mer jobb att skriva, läsa och underhålla koden, och riskerar att ändringar inte genomförs konsekvent på alla platser där koden upprepas. Det är viktigt att man redan tidigt lär sig tänka på detta och lär sig skriva kod på ett effektivt sätt – att lära sig bra tekniker kan spara mycket tid i längden!

Man kan ofta undvika onödig repetition genom hjälpmetoder (anropas flera gånger med olika parametrar), abstrakta hjälpklasser (samlar gemensam implementation inom en arvshierarki), eller liknande. Vet du inte hur du ska göra: Fråga innan du lämnar in!

Notera att detta inte är något vi ignorerar. Det är vanligt med komplettering för upprepad kod, så gå genom exemplen nedan (och kom ihåg att det finns fler varianter av repetition än dessa).

1. #radupprepningUndviker du att upprepa mer eller mindre identiska rader?

   Även om raderna inte är helt identiska, för att de använder olika värden på några platser, kan man bryta ut dem till en metod som anropas flera gånger med olika parametrar.

   RepeatedExpression

   SimilarBranches

   SimilarConditionals

   SimilarNestedClasses

2. #deluttryckGer du namn till deluttryck som används flera gånger?

   Om abs(mouseEvent.getX() - (BUTTON_X + BUTTON_WIDTH / 2)) används mer än en gång: Tilldela värdet till en lokal variabel (som kan deklareras final), och använd denna. Detta kan förbättra läsbarheten förvånansvärt mycket.

   På motsvarande sätt kan följande:

   p.setxDir(Calc.calculateDirection(ss.getX(), ss.getY(), toX, toY)[0]);
   p.setyDir(Calc.calculateDirection(ss.getX(), ss.getY(), toX, toY)[1]);
   	    

   ...gärna bytas ut mot:

   double[] direction = Calc.calculateDirection(ss.getX(), ss.getY(), toX, toY)
   p.setxDir(direction[0]);
   p.setyDir(direction[1]);

   Därifrån kan man så klart gärna gå vidare och införa en klass för riktningar, istället för en array där [0] och [1] inte är meningsfulla index:

   Direction direction = Calc.calculateDirection(ss.getX(), ss.getY(), toX, toY)
   p.setxDir(direction.x);
   p.setyDir(direction.y);

   Att bryta ut variabler kan man göra även för deluttryck som bara används en gång, vilket diskuteras under självförklarande kod.

   RepeatedExpression

3. #samlingar Använder du listor istället för multipla variabler?

   Vi ser ofta kod där man skapar flera olika variabler istället för att använda en enda lista. Detta kan verka väldigt frestande när man bara har 2 eller 3 objekt av en viss typ, t.ex. spelare:

   
   	      public class Game {
   	          private Player player1;
   	          private Player player2;
   	          // ...
   	      }
   	  

   Det kan också gälla att man har exakt 2 färger (svart och vit) i schack, att man har exakt 4 banor i ett spel, eller liknande. Med ett litet konstant antal blir det frestande att skapa separata variabler för var och en, precis som ovan. Då kanske man också får metoder som getPlayer1() och getPlayer2().

   Problemet är att man ofta vill göra något med alla spelare, färger, eller banor. Det är mycket lättare att göra det om man har en lista ("för alla spelare i listan") än när man har två separata variabler. Annars blir det lätt en del upprepad kod, där man skriver om samma sak för player1 och player2.

   
   	      public class Game {
   	          private List<Player> players;
   
   	          private void tick() {
   	              for (Player player : players) { ... }
   	          }
   	          public Player getPlayer(int index) {
   	              return players.get(index);
   	          }
   	      }
   	  

   Eller så vill man slumpa fram en bana från de banor man har. Detta är enkelt om man bara kan slumpa fram ett index i listan på banor, och plocka ut levels.get(index). Om varje bana är en egen variabel får man istället en lång switch- eller if-else-sats: if (level == 3) return level3; else....

   Där ser vi också varför detta inte bara är en fråga om hur mycket kod vi skriver utan om modellering. Har vi en uppsättning av någonting vill vi kunna referera till detta som en uppsättning, till exempel listan av spelare. Med separata variabler för varje spelare missar vår modell kopplingen mellan dem: Att de hör ihop.

4. #metodlikhet Kombinerar du metoder som gör nästan samma sak?

   Om två metoder gör nästan samma sak (har nästan samma ansvarsområde): Se om det är lämpligt att kombinera dem till en metod, där en parameter avgör vilken variant som utförs. Övning 4.2 visade en specifik variant av detta generella problem.

   En variant av detta kan uppstå i t.ex. schackspel: För många olika pjästyper kan det till exempel finnas behov av att kontrollera hur långt man kan gå i en viss riktning. Även här kan en riktning anges av en enum-klass (Direction med 8 värden för de 8 raka och diagonala riktningarna), och man kan ha en enda kontrollmetod som stegar enligt det deltaX och deltaY som anges av Direction-objektet!

5. #abstraktklass Har du infört abstrakta klasser där detta passar?

   Om flera klasser lagrar samma typ av information, och dessa klasser "hör ihop" begreppsmässigt: Du behöver kanske en abstrakt klass som kan lagra den gemensamma informationen.

   Exempel: Player, Ball och Powerup lagrar x/y-positioner och har getters och setters för dessa. Vi inser att anledningen till likheten är att de alla är entiteter som ska visas på skärmen, vilket är ett intressant begrepp att införa. Därför adderar vi den abstrakta klassen ScreenEntity som representerar detta begrepp och innehåller den gemensamma funktionaliteten.

   SimilarChildren

6. #hierarkinivå Där du har typhierarkier, har du samlat kod på rätt nivå?

   Om samma eller liknande metodimplementation repeteras flera gånger, men i olika klasser i en hierarki: Ofta ska man ha en gemensam metod i en gemensam superklass. Detta kan i vissa fall vara en abstrakt klass. Exempel: Flera relaterade klasser har (nästan) samma kod för kollisionshantering.

   SimilarChildren

7. #datasomkod Använder du filer för att läsa in större mängder programdata?

   Det är till exempel möjligt att definiera spelbanor genom kod som konstruerar dem steg för steg, men om detta leder till 300 rader av kod som skapar "new Obstacle()" och placerar dessa objekt på rätt plats, kommer denna kod inte att räknas i projektet. Dessutom kan man fundera på om man istället ska skriva kod som läser in banor från en datafil, vilket ger mer flexibilitet för användaren att skapa egna banor. Inläsning från fil gjordes bland annat i en uppgift i Tetris, betyg 4 (användning av Gson).

   DataAsCode

8. Använder du enklaste sättet att få tag på data i ett objekt?

   En enkel getter är en metod som helt enkelt returnerar värdet på ett fält. Det är bra att använda sådana metoder för att undvika behovet av publika fält, eftersom publika fält gör att andra klasser får reda på den interna representationen. Men inom klassen där fältet deklareras finns mindre behov av detta, och där är det bättre att använda fältet direkt för att få kortare och mer lättläst kod.

   Call to simple getter '...' from within class varnar för detta.

9. #idearep Har du använt IDEA för att hitta vissa former av repetition?

   Med hjälp av Analyze | Locate Duplicates kan man hitta vissa former av potentiellt repetitiv kod. Den kan ge många falska varningar, och missar mycket av vad som diskuteras ovan – men den är ändå potentiellt hjälpsam.

10. #klasskombination Kombinerar du klasser som gör nästan samma sak?

    Om två klasser gör nästan samma sak (har nästan samma ansvarsområde): Se om det är lämpligt att kombinera dem till en klass, där en (konstruktor)parameter avgör skillnaden. Ett exempel på detta kan finnas i händelsehantering i GUI. Se till exempel följande klasser:

    private class UpAction extends AbstractAction
    {
        @Override public void actionPerformed(final ActionEvent e) {
    	game.move(Direction.UP);
        }
    }
    
    private class RightAction extends AbstractAction
    {
        @Override public void actionPerformed(final ActionEvent e) {
    	game.move(Direction.RIGHT);
        }
    }
    
    private class DownAction extends AbstractAction
    {
        @Override public void actionPerformed(final ActionEvent e) {
    	game.move(Direction.DOWN);
        }
    }
    
    private class LeftAction extends AbstractAction
    {
        @Override public void actionPerformed(final ActionEvent e) {
    	game.move(Direction.LEFT);
        }
    }

    Här kan samtliga klasser lätt ersättas med en:

    private class DirectionAction extends AbstractAction
    {
        private Direction dir;
        public DirectionAction(Direction dir) {
    	this.dir = dir;
        }
        @Override public void actionPerformed(final ActionEvent e) {
    	game.move(dir);
        }
    }

    Detta har också den stora fördelen att man inte behöver skapa nya sådana klasser om man inför nya riktningar (Directions). Istället kan samma klass återanvändas för samtliga riktningar.

    Om det till att börja med ser ut så här, då?

    private class UpAction extends AbstractAction
    {
        @Override public void actionPerformed(final ActionEvent e) {
    	game.moveUp();
        }
    }
    
    private class RightAction extends AbstractAction
    {
        @Override public void actionPerformed(final ActionEvent e) {
    	game.moveRight();
        }
    }
    
    private class DownAction extends AbstractAction
    {
        @Override public void actionPerformed(final ActionEvent e) {
    	game.moveDown();
        }
    }
    
    private class LeftAction extends AbstractAction
    {
        @Override public void actionPerformed(final ActionEvent e) {
    	game.moveLeft();
        }
    }

    Då går det inte lika enkelt att byta ut koden, eftersom det är olika metoder som anropas. Men då börjar problemet helt enkelt ett steg tidigare: Man behöver oftast inte olika metoder för att flytta sig i olika riktningar, så man borde börja med att införa move(Direction), och sedan fortsätta enligt exemplet ovan.

Självförklarande kod

Mycket kan vinnas i fråga om läsbarhet om man siktar på att skriva självförklarande kod. Ju tydligare koden själv visar vad den gör och varför, desto färre kommentarer behövs. Då minskar också risken att man glömmer uppdatera en kommentar när koden ändras!

Här är det väldigt viktigt att man inför lämpliga abstraktioner, som låter koden säga vad som faktiskt menas på en högre nivå. Eloquent Javascript ger ett träffande exempel som vi kan återge i modifierad form:

int total = 0, count = 1;
while (count <= 10) {
  total += count;
  count += 1;
}

Denna kod är ganska kort och vi kan ganska lätt lista ut vad den gör: Summerar talen från 1 till 10. Men tänk om vi inför funktionen range:

int total = 0;
for number in range(1, 10) {
  total += number;
}

Då behöver vi inte längre lista ut vad loopen egentligen gör med count-variabeln utan kan börja prata om intervall av tal. Och om vi inför funktionen sum:

int total = sum(range(1, 10)); 

Då kan vi direkt se vad koden gör, utan att lista ut att den räknar ut en summa. Här blir vi alltså hjälpta av att införa nya begrepp som vi kan använda för att beskriva vad programmet ska utföra. Sedan kan det hända att programmet totalt sett blir längre när man tar med definitionen av sum och range, men det blir ändå lättare att förstå.

Nedan diskuterar vi bland annat att införa begrepp och att ge dem informativa namn.

1. #variabelnamn Används informativa och rättvisande namn på variabler, inklusive parametrar och fält?

   Då minskar behovet av dokumentation. Exempel:
   int i = 10; // Number of pixels to move for each step

   Förbättrad namngivning (som också gör det lättare att förstå vad som händer på alla ställen som faktiskt använder variabeln):
   int pixelsPerStep = 10;

   En lista över uppkopplingar i en nätverksklient kan heta connections eller kanske currentConnections, medan c inte är informativt och windows är helt missvisande, även om varje uppkoppling har ett eget fönster på skärmen.

   Tänk dig att några andra kursdeltagare för första gången skulle titta på en enskild metod i din kod. Skulle de förstå vad namnen betyder? Ger namnen tillräcklig information utan att man redan vet allt i förväg, som du själv gör? Är namnen otvetydiga, svåra att missförstå även om man "försöker"?

   Samlingar av objekt namnges normalt i plural, utan att man anger typ av samling: List<Connection> connections; istället för List<Connection> connectionList;. Eftersom namnet är plural framgår det redan att det handlar om en samling.

2. #metodnamn Används informativa och rättvisande namn på metoder?

   Precis som för variabler minskar detta behovet av dokumentation. Exempel: En metod som flyttar alla fiender i ett spel kan gärna heta moveEnemies(), medan update() visserligen stämmer men inte är så informativt.

   Metodnamn är normalt verb, eftersom metoder säger till objekt att göra något. Till exempel döper man normalt inte en metod till fileReader() eller fileReading() utan till readFile().

   Boolean method name '...' does not start with question word – Det är bra om metoder som ger ett ja/nej-svar börjar med ett frågeord. Till exempel använder man enligt Java-standard window.isVisible() istället för window.getVisible(), och colorModel.hasAlpha() istället för colorModel.getAlpha().

   Suspicious getter / setter – Java har en standard för get/set-metoder, där t.ex. värdet på fältet foo hämtas av getFoo() och sätts av setFoo(). Om man inte följer detta mönster får man en varning.

3. #klassnamn Används informativa och rättvisande namn på klasser och gränssnitt?

   Då minskar behovet av dokumentation. Till exempel är Info och Data inte så informativa namn, då i princip alla objekt innehåller information / data.

   Tänk på att klassnamn normalt är substantiv i singular: Tower, inte Towers. Därmed gör man t.ex. "new Tower()" (ett nytt torn), inte "new Towers()" (flera nya torn?).

4. #begreppsnamn Har du infört namngivna begrepp när detta underlättar läsbarhet?

   Både metoder och lokala variabler kan användas för att ge meningsfulla namn och göra koden självförklarande. Vi ger flera exempel, eftersom detta är viktigt:

   1. Från Best practices for commenting your code: Använd meningsfulla identifierare och konstanter, även om de bara används en gång!

      // Before
      // Calculate monkey's arm length
      // using its height and the magic monkey arm ratio
      double length = h * 1.845; //magic numbers are EVIL!

      // After - No comment required
      double armLength = height * MONKEY_ARM_HEIGHT_RATIO;

      Liknande exempel: Även korta uttryck som Math.abs(x1-x2), som används en eller flera gånger i ett större uttryck, kan tilldelas till en egen namngiven variabel innan de används: int deltaX = Math.abs(x1-x2). Detta förklarar direkt vad deluttrycket innebär.

   2. Anta att man implementerar ett schackspel. Tekniskt sett går det utmärkt att testa om vissa koordinater ligger inom spelbrädets ramar på följande sätt:
      1 <= x && x <= board.getWidth() &&
1 <= y && y <= board.getHeight()

      Men om vi inför metoden contains(x,y) i spelbrädet, blir det lättare att läsa, förstå och kontrollera korrektheten:
      board.contains(x,y)

      Detta gäller även om uttrycket bara används en enda gång – för genom att införa namnet "contains" förklarar koden på en högre nivå vad den egentligen försöker åstadkomma!

   3. I samma schackspel: Tekniskt sett går det utmärkt att kolla om det finns en pjäs på en viss position genom att testa om board.getPiece(x,y) == null. Men genom att införa board.isEmptyAt(x,y) inför vi ett nytt intressant begrepp, och vi slipper bry oss om implementationsdetaljen att "null" innebär en tom ruta.

   4. Kod som testar om (player.getRightEdge() - offset >= monster.getLeftEdge()) && player.getRightEdge() - offset <= monster.getRightEdge() blir lättare att läsa om vi inför metoden between(lower, value, upper), och anropar between(monster.getLeftEdge(), player.getRightEdge() - offset, monster.getRightEdge(). Inte bara för att det blir lite mindre kod att läsa, utan för att vi direkt ser att vi testar om ett värde är mellan två andra.

5. #magiskt Är programmet fritt från magiska konstanter?

   Exempel: Ser man width/30 vet man kanske inte vad 30 betyder. Följande är lättare att förstå:

   public final static int BLOCK_SIZE = 30;
   ...width/blockSize...

   I vissa fall är konstanter av en mer lokal natur. Då går det bra att istället använda en "lokal konstant" för att definiera ett namn:

   final int blockSize = 30;
   ...width/blockSize...

   Bra namngivning är ett bra sätt att göra koden mer självförklarande och minska behovet av kommentarer.

   Exempel 2:
   

   // Shuffle the deck (standard decks have 52 cards)
   for (int i = 1; i <= 52; i++) {
       // 53 is the deck size + 1
       deck.swap(i, i + randomInt(53-i) - 1);
   } 

   Inför vi en namngiven konstant blir 52/53 självdokumenterande. Vi kan använda en lokal konstant (final int), eller deklarera konstanten i klassen (till exempel private final static int DECK_SIZE = 52;). Då blir det också enkelt att ändra värdet vid behov. Här kunde man annars ändra alla "52" men glömma "53" – eller råka ändra värdet 52 på en plats där det egentligen var en skärmkoordinat istället för antal kort!

   // Shuffle the deck
   final int deckSize = 52;
   for (int i = 1; i <= deckSize; i++) {
       deck.swap(i, i + randomInt(deckSize+1-i) - 1;
   }

   Magic Number – Du använder magiska tal. Detta upptäcker inte andra typer av konstanter.

   Det finns vissa undantag där magiska tal kan användas utan att förvirra. Ett sådant exempel är för färger, där "new Color(0,255,0)" knappast behöver namngivna konstanter för värdena 0 och 255. Där deklarerar du istället själva Color-objekten som final-"konstanter", så kommer IDEA inte att klaga.

6. #enum Används enum-typer överallt där en variabel kan ha ett fåtal förutbestämda värden?

   Uppräkningsbara typer (enum) är ett utmärkt verktyg för att se till att en variabel verkligen bara kan få rätt värden, som dessutom inte kan blandas ihop med värden av andra typer.

   Med andra ord, undvik t.ex. String direction="up"; // eller "down", "left", "right" och int state = 1; // eller 2...7. Med en enum kan man bara ange korrekta värden (exempel: mode = NORMAL, INVISIBLE, GHOST).

   Att inte använda enum leder ofta till följdproblem och komplettering.

7. #förenkling Har du försökt förenkla kod som kan vara svår att förstå?

   Tänk på att även andra ska kunna läsa koden. Förenklingar bör vara det första steget, innan man väljer att dokumentera.

8. #namnstandard Används Javas namnstandard?

   Detta har varit en gemensam standard sedan över 20 år, och underlättar för Javaprogrammerare att läsa varandras kod.

   • class KlasserHarStorBokstav (med namn i singular: Tower, inte Towers)
   • fältOchVariablerBörjarMedLitenBokstav
   • metoderBörjarMedLitenBokstav()
   • public static final int GLOBALA_KONSTANTER_HETER_SÅ_HÄR
   • package se.liu.ida.jonkv82.utan.stora.bokstäver;

   Package naming convention – ska tala om när ett paketnamn (package xyz;) bryter mot namnstandarden. Fråga handledaren om du är osäker på anledningen.

   Det finns flera inspektioner som verifierar andra aspekter av namnstandarden.

9. #borrowedcode Visar själva koden vilka delar som är lånade?

   Under Samarbete, fritt material och fusk visar vi hur du ska döpa lånade metoder och kodsnuttar så att det framgår, även för automatiska analysverktyg, att dessa är lånade.

   Var väldigt försiktig med detta, så du inte misstänks för fusk!

Dokumentation och kommentarer

Som du kan se från föregående avsnitt satsar vi mycket på att göra koden mer lättläst, för att behovet av dokumentation och kommentarer ska minska, men det betyder inte att behovet helt försvinner.

1. #strukturdok Har du skrivit övergripande dokumentation för programmets struktur?

   För projektet ska detta finnas i projektrapporten, eftersom man ofta behöver läsa dokumentationen för att överhuvudtaget veta vilka klasser man är intresserad av, om den ens är relaterad till en specifik klass.

   Ett par förkortade exempel på vad som kan passa in här: "Alla rörliga objekt på skärmen är subklasser till Mover. Vill man ha flygfunktionalitet kan man ärva från Flier istället, eftersom detta har inbyggda funktioner för att landa. Varje rörligt objekt har en egen bakgrundstråd och rör sig individuellt med synkronisering via... /alternativt/ Vi använder en centraliserad spelloop som med jämna mellanrum anropar samtliga rörliga objekts uppdateringsmetod så de kan röra sig ett steg. Objekt som vill uppdateras mer sällan kan ange detta genom..."

2. #javadocklass Har varje klass en beskrivande Javadoc-kommentar?

   Javadoc-kommentarer är en standard. Det kan hända att bara 2-3 meningar behövs, men minst så mycket bör man i alla fall kunna skriva om de flesta klasser.

   Javadoc är tänkt att kunna plockas fram automatiskt. I IDEA görs detta genom att man ställer markören på det man vill veta mer om (klassnamn, fältnamn, metodnamn, ...) och trycker Ctrl-Q för Quick Documentation. Om man inte får fram dokumentationen på det sättet har man inte skrivit den på rätt sätt. Det räcker alltså inte att det ligger någon sorts kommentar "i närheten" i källkoden, för verktygen ska kunna hitta dokumentationen med rätt format.

   Declaration has Javadoc problems visar bland annat om en Javadoc-kommentar helt saknas för en klass – men den kan inte visa om kommentaren innehåller meningsfull information, vilket också ska finnas!

3. #javadocfält Har varje publikt fält en beskrivande Javadoc-kommentar?

   Fält ska oftast inte vara publika (vilket diskuteras på annan plats i dokumentet), men om de i undantagsfall kan accepteras måste de dokumenteras med en fullständig Javadoc-kommentar. Javadoc beskrivs mer under #JAVADOCKLASS ovan.

   Declaration has Javadoc problems visar bland annat om en Javadoc-kommentar helt saknas för ett fält – men den kan inte visa om kommentaren innehåller meningsfull information, vilket också ska finnas!

4. #lättförståelig Är koden tillräckligt lättförståelig och kommenterad i övrigt?

   Tänk på att handledaren och examinatorn ska läsa genom hela projektet!

5. #kommentarstyp Ger kommentarerna rätt typ av förklaringar?

   Det är lätt hänt att man skriver en enkel förklaring av vad koden gör, steg för steg – ibland är detta motiverat, men ibland leder det helt enkelt till en duplicering av information som man lätt kunde ha fått genom att läsa själva koden istället. Med andra ord, kommentera inte det som är fullständigt uppenbart:

     windows.add(myNewWindow); // Add the window to the list

   Kommentarer som förklarar varför man gör något eller varför man gör det just på detta sätt är ofta mer värdefulla, liksom kommentarer som förklarar koden i ett större sammanhang. Denna typ av kommentarer är ofta svårare att ersätta med lättläst kod.

Korrekt objektorientering

1. #infoklass Lagras information i rätt klass?

   Ofta är det tydligt var information hör hemma. Om ett spelarobjekt kan rita ut sig själv på skärmen, så är det lämpligt att spelarbilden lagras just i detta objekt, och att det är spelarobjektet som håller reda på hur stor bilden är. Vi har sett exempel där detta ansvarsområde delas upp mellan flera olika klasser (till exempel genom att bildhöjden är hårdkodad i en annan klass), vilket oftast inte är korrekt.

2. #livstid Lagras information med rätt livstid?

   Använd till exempel inte fält i onödan. Om information bara behöver lagras under ett visst metodanrop och ingen återanvänder den efter att metoden avslutas, är det bättre att lagra den informationen i en lokal variabel. Annars riskerar man till exempel att flera metodanrop (parallella eller rekursiva) skriver över varandras värden då de använder samma objekt och alltså samma fält.

3. #initfält Instance field may not be initialized

   Alla fält ska initialiseras (ges ett värde) av konstruktorn. Om de ska ha sitt defaultvärde (0, 0.0, false, null) ska de ändå initialiseras uttryckligen, så att det tydligt syns att det är meningen att de har defaultvärdet snarare än att man har glömt initialiseringen.

   Det är OK att initialisera värdet vid deklarationen istället för i konstruktorn:
   private int xpos = 0;

4. #barakonstrueraAnvänder du konstruktorer bara för att konstruera?

   Konstruktorer bör användas för att konstruera ett objekt och ge dess fält initialvärden. Det kan vara frestande att lägga mer kod i konstruktorn för att man alltid vill köra den så snart ett objekt har konstruerats:

   public class Game {
       public Game() {
           // ... construct ...
           // ... set field values ...
           // ... start game thread ...
       }
   } 

   Men konstruktorn ska användas för att konstruera objektet, se till att fält sätts till rimliga värden. Sedan ska vi anropa andra metoder för att göra saker med objektet. I detta fall skulle man sannolikt vilja ha en separat metod för att starta en bakgrundstråd för spelet, så att vi har två separata operationer:

   • Skapa ett spelobjekt som håller reda på speldata.

   • Starta en tråd så att spelet kör igång.

   Samma gäller här:

   public class StrongPowerup {
       public StrongPowerup(Player player) {
           // ... construct ...
   	player.setStrength(100);
       }
   } 

   Det vore bättre att istället ha en separat metod apply(Player player), så man frikopplar skapandet från användandet.

   Ett tredje exempel:

   public class Dialog {
       public Dialog() {
           // ... construct GUI, etc ...
           // ... ask user a question ...
       }
   } 

   Här vill vi istället att dialogklassens konstruktor skapar dialogobjektet (lagrar relevanta data i relevanta fält) och att en separat metod sedan anropas för att visa dialogen så att användaren kan svara på frågan.

5. #staticvariabler Undviker du static-variabler eller använder du dem korrekt (undantagsfall)?

   Static-variabler (till skillnad från globala konstanter) har primärt ett acceptabelt användningsområde: Lagring av data som måste vara unika/globala, och som inte kan skickas med till alla som behöver dessa data. Varje enskild static-variabel måste därför motiveras tydligt:

   • Varför är det viktigt att just denna information bara kan lagras en gång, i ett globalt tillstånd?

   • Varför skulle det vara alltför problematiskt att lagra just denna information i ett objekt som skapas en gång och skickas med till de som behöver den?

   Ett spelexempel:

   • Lagrar du data som statiska variabler, kanske för att andra lättare ska få tag på dem?

     public class GameBoard {
         public static Player player1;
         public static Player player2;
         public static List<Monster> Monsters;
         public static GameState currentState;
     } 

     Tänk om! Vi håller på med objektorientering, och data ska lagras i objekt så långt det går.

     I framtiden kan du dessutom vilja göra om ditt projekt för att spela flera parallella spel, och det går inte om du har deklarerat att du definitivt bara har ett currentState för alla pågående spel. Även om du vet att du inte vill ha flera åt gången, kommer vi att slå ner på statiska (globala) variabler då det leder till många andra problem: Icke-lokalitet, brist på kontroll över vem som använder informationen, implicita kopplingar mellan olika delar av ett system, problem vid multitrådad programmering, med mera. Det är bättre att från början lära sig att undvika statiska variabler, och senare (med mer erfarenhet) förstå de begränsade fall när de faktiskt är motiverade.

     Undantaget är (1) namngivna konstanter, som public static final int SIZE = 20, och (2) data som faktiskt gäller själva klassen, som vårt tidigare exempel private int circlesCreated = 0;

   • Skapar du ett globalt tillgängligt GameBoard istället?

     public class GameBoard {
         public Player player1;
         public Player player2;
         public List<Monster> Monsters;
         public GameState currentState;
         public static GameBoard theOneAndOnly = new GameBoard();
     } 

     Tänk om! Du har kommit en bit på vägen, men i och med att du har ett enda unikt GameBoard har du ändå nästan alla problem som vi diskuterade nyss. Skicka istället med GameBoard till de som behöver den – i vissa fall innebär det att du skickar GameBoard till ett annat objekts konstruktor så att objektet permanent kan känna till Gameboard, men ofta är det bättre att istället skicka med GameBoard till de metoder som behöver det just då.

   • Använder du public static HighScoreList board = new HighScoreList(); för att det är bekvämt att alla får tillgång till listan utan att du behöver ge den till dem? Om du är helt säker på att du aldrig vill ha flera listor, och det är krångligt att skicka med listan till de som behöver den, kan detta vara acceptabelt.

     Men är det verkligen krångligt? Är det inte egentligen så att listan kunde skapas på ett ställe (spelet) och möjligen skickas med i ett par metodanrop till något enstaka ställe där den också behövs? I så fall har vi ju inget behov av en global variabel (static), utan kan använda en helt vanlig variabel istället! Spelklassen kan ju lätt se till att inte skapa två listor, så det finns ingen risk att "råka" få flera när man bara vill ha en.

   StaticNonFinalField –

   PublicStaticCollectionField –

   PublicStaticArrayField –

6. #singleton Undviker du Singleton-mönstret, eller använder du det korrekt (undantagsfall)?

   Singleton-mönstret används för att kunna komma åt ett unikt objekt av en viss klass från godtycklig punkt i programmet. Det delar därför många av nackdelarna med static-variabler, som diskuterades i förra punkten. Detta vill vi oftast undvika. Singleton ses ibland som ett "anti-mönster" som används alltför ofta!

   Varje användning av Singleton måste motiveras tydligt: Varför är det viktigt att informationen bara kan lagras en gång, i ett globalt tillstånd?

   Singleton –

   GetInstanceIndicatesSingleton –

   PseudoSingleton –

7. Refused bequest

   "Bequest" är en testamenterad gåva, dvs. ett arv. Varningen betyder att en metod foo() "override:ar" en annan, utan att anropa den ursprungliga implementationen, super.foo(). Detta kan vara korrekt (om man faktiskt vill ignorera superklassens implementation) men kan också leda till buggar om man bara ville "utöka" superklassens funktionalitet. Om en GUI-komponents paintComponent()-metod glömmer att anropa super.paintComponent() kommer den t.ex. inte automatiskt att rita ut sin bakgrundsfärg.

   Om du anser att det är korrekt att ignorera superklassens implementation motiverar du som vanligt detta i en kommentar.

Ärvning, polymorfism med mera

1. #användhierarkier Använder du typhierarkier där detta är rimligt?

   Om flera klasser hör ihop begreppsmässigt (Rectangle, Circle Octagon) kanske de bör ha en gemensam supertyp som reflekterar vad som är gemensamt för dem (Shape). Detta kan till exempel vara ett gränssnitt (interface).

2. #undvikhierarkier Låter du bli typhierarkier där dessa inte behövs?

   Det kan hända att det som nu är flera olika klasser egentligen borde vara samma klass. Se föreläsningsbilderna, avsnittet om ärvning, "Fördelar och nackdelar!". Där finns exempel på en persondatabas där det visade sig orimligt att ha en typhierarki med en klass för varje "typ" av person, och bättre att ha en enda klass för alla personer.

   Se även Ärvning: Undvik onödiga underklasser i samma föreläsning. Vi ska inte skapa nya klasser bara för att det går, utan för att vi faktiskt behöver modellera något som är så annorlunda att det inte passar in i en existerande klass. Om bara data skiljer sig åt (t.ex. styrkan på en spelare) är det nästan alltid bättre att göra detta till en parameter, och att inte skapa några subklasser. Konkreta exempel finns i föreläsningsbilderna. Detta är en relativt vanlig anledning till komplettering.

   MaybeUnnecessarySubclass –

   SimilarChildren –

3. #komposition Gör du rätt val mellan typhierarkier och komposition?

   Är den eller har den? Om B är subtyp till A, så ska B vara en speciell sort/variant av A, det vill säga "alla B är A" (alla cyklar är fordon).

   Vi ska alltså inte låta BankAccount ärva från ArrayList bara för att det är ett bekvämt sätt att kunna lagra en lista av transaktioner. Ett bankkonto är inte "en speciell sorts lista"! Istället är det bättre att bankkontot har en lista – komposition istället för ärvning, som diskuteras i föreläsningsbilderna.

   En praktisk anledning till att undvika detta, utöver argumentet att ett bankkonto inte är en sorts lista, är att när man ärver från en klass gör man också alla den klassens publika och "protected" metoder tillgängliga för andra. Det blir alltså möjligt att manipulera bankkontot med alla listmetoder. Detta kan vara farligt och/eller förvirrande (när man blir överväldigad av alla ArrayList-metoder som inte är relevanta för bankkontot). Även av den anledningen kan det vara bättre att bankkontot har en lista.

   Att man ärver superklassens metoder innebär också att man får se upp om superklassen ändras och lägger till nya metoder, kanske med samma namn och parametertyper som dina egna metoder.

   Detta betyder så klart inte att ärvning alltid är dåligt – ärvning har viktiga användningsområden, men är inte lösningen på alla problem.

4. #initialiseraegna Låter du klasser initialisera sina egna objekt?

   Följande exempel är felaktigt:

   public class Circle {
      public double x, y, r;
   }
   public class GraphicCircle extends Circle {
       private Color outline, fill;
       public GraphicCircle(double x, double y, double r,
           Color outline, Color fill) 
       {
           this.x = x;
           this.y = y;
           this.r = r;
           this.outline = outline;
           this.fill = fill;
       }
   }

   Anledningen är att Circle inte får en chans att själv initialisera sina fält x, y och z med hjälp av sin egen konstruktor. Detta bryter fullständigt mot objektorienteringens principer och ger normalt komplettering. Se föreläsningsbilderna!

   Följande är precis lika felaktigt:

   public class Entity {
      public int strength;
   }
   public class StrongEnemy extends Entity {
       public StrongEnemy() 
       {
           this.strength = 10;
       }
   }

   Även i detta fall tilldelar man ju ett värde till superklassens fält i subklassens konstruktor.

   Constructor assigns value to field defined in superclass upptäcker detta.

5. #typkontroll Undviker du att göra "egen" typkontroll? Använder du polymorfism där detta är rimligt?

   (Se även föreläsningen om ärvning, avsnitt "Egen typkontroll i Java".) Vi ser ibland projekt där man använder olika sätt att ta reda på vilken typ ett visst objekt har inom en typhierarki, och agera olika beroende på typen:

   • if (x instanceof Monster) { monsterCollision(); }
else if (x instanceof Player) { playerCollision(); }

   • if (x.getClass() == Monster.class) { monsterCollision(); }
else if (x.getClass() == Player.class) { playerCollision(); }

   • if (x.getType() == Type.MONSTER) { monsterCollision(); }
else if (x.getType() == Type.PLAYER) { playerCollision(); }

   • if (x.isMonster()) { monsterCollision(); }
else if (x.isPlayer()) { playerCollision(); }

   • ...

   IDEA varnar för vissa av dessa men inte för alla "workarounds", till exempel de rader som visserligen undviker "instanceof" men ändå gör i princip samma sak: Inspekterar objektets typ och agerar därefter.

   Det är just detta som subtypspolymorfism är till för! I 999 fall av 1000 är det bättre att låta Monster och Player ha en gemensam supertyp, till exempel GameObject. Låt GameObject ha en abstrakt collision()-metod. Låt Monster och Player implementera den på varsitt eget sätt: Monster gör monsterCollision(), medan Player gör playerCollision(). Och istället för att göra en test som i exemplet ovan, anropa helt enkelt x.collision(). Då ser den dynamiska bindningen till att rätt implementation används.

   Behövde du någon information som bara fanns tillgänglig i klassen där testet fanns? Skicka då med den som parametrar till din nya collision()-metod!

   Användning av instanceof, eller liknande konstruktioner, är en relativt vanlig källa till kompletteringar!

   Obs: Detta gäller även om det bara är en enda test som görs: "Om det är ett monster, gör så här. Annars gör vi inget." – Detta är också en typkontroll som kunde hanteras genom att monstrets metod gör "så här", och andra klasser har tomma metoder som inte gör något alls.

   Obs: Det finns vissa situationer där detta kan vara svårare att införa än annars, till exempel där det man ska göra beror på typen hos två olika objekt, inte bara ett (x ovan). Om du tror du har ett sådant fall: Fråga examinatorn i god tid innan inlämning!

   Chain of 'instanceof' checks – Koden testar explicit vilken typ ett objekt har, t.ex. om det är en Circle, Rectangle eller Square, och gör sedan olika saker beroende på typen. Inspektionen upptäcker bara vissa specifika fall!

   'instanceof' check for 'this' – Koden testar explicit vilken typ detta objekt har. Det är extremt ovanligt att detta är korrekt: Använd polymorfism och overriding istället.

   InstanceofOutsideEquals –

6. #abstraktabstrakt Är eventuella abstrakta klasser verkligen abstrakta?

   En klass behöver bara vara abstrakt om man vill ange att alla subklasser ska ha vissa metoder, men inte inte kan ge en "standardimplementation" för alla dessa metoder. Abstrakta klasser ska alltså normalt ha abstrakta metoder. Om alla metoder har en implementation gäller något av följande:

   • Klassen är fullt funktionsduglig i sig själv och behöver inte vara abstrakt.
   • En eller flera av de tomma metoderna har en orimlig implementation som gör att klassen inte är fullt funktionsduglig. Den implementationen borde i de flesta fall tas bort och ersättas med en abstrakt metod.

   Abstract class without abstract methods varnar för detta.

   No-op method in abstract class / No-op method '...' should be made abstract – Om man ger en tom metodimplementation i en abstrakt klass är det lätt att missa att implementera metoden "på riktigt" i en subklass. Normalt vill man ha en icke-implementerad metod i den abstrakta klassen istället, dvs. "method();" istället för "method() {}". Då klagar kompilatorn om man glömmer att implementera den i en konkret subklass.

7. #staticfråninstans Access static member via instance reference /
   Static member '...' accessed via instance reference

   Du använder en statisk metod via en objektinstans, antingen explicit (object.staticmethod()) eller implicit (staticmethod() anropad från en icke-statisk metod i samma klass). Detta är missvisande, eftersom det ser ut som om man använder en vanlig instansmetod. Kvalificera med klassnamnet: classname.staticmethod().

8. Assignment to static field '...' via an instance

   Du använder ett statiskt fält via en objektinstans, antingen explicit (object.staticfield) eller implicit (staticfield används från en icke-statisk metod i samma klass). Detta är missvisande, eftersom det ser ut som om du använder ett vanligt instansfält som har ett värde för varje instans, medan det i själva verket bara lagras ett värde för hela klassen. Kvalificera med klassnamnet: classname.field.

Inkapsling och synlighet

1. #gömfält Gömmer du fält?

   Publika fält ska normalt inte användas, eftersom det gör att man exponerar interna implementationsdetaljer för andra. När andra klasser direkt använder de publika fälten blir det svårt att ändra den interna datarepresentationen.

   Vissa mycket ovanliga undantag kan göras, främst för mycket enkla datalagringsklasser som t.ex. Rectangle i Java. Dessa har i princip inget eget beteende överhuvudtaget, och det är extremt osannolikt att man någonsin skulle vilja ändra den interna definitionen av en rektangel. Fråga om du är osäker.

   Public field varnar för detta.

2. Har abstrakta klasser skyddade konstruktorer?

   Man kan inte kan skapa en instans av en abstrakt klass, utan bara instanser av dess subklasser. Därför är det onödigt att konstruktorer i abstrakta klasser är public. Det räcker gott med att de är protected.

   Public constructor in abstract class varnar för detta.

3. #paketsynlighet Undviker du att göra medlemmar "paket-synliga"?

   Klasser, metoder och fält som varken deklareras public, protected eller private blir paketsynliga, alltså åtkomliga från alla klasser inom samma paket. Detta kan vara motiverat i vissa ganska sällsynta fall, men är oftare ett tecken på problematisk modellering där man har tagit hänsyn till implementationsdetaljer istället för hur klasserna egentligen borde modelleras. Det kan också indikera att man helt enkelt har glömt bort att ange en synlighetsnivå och att fundera över vilken nivå som kan vara rimlig.

   Om det handlar om information och funktionalitet som ska döljas, av säkerhetsskäl eller för att förenkla framtida ändringar, är private bättre då vem som helst ändå (normalt sett) kan lägga klasser i "ditt" paket och därmed komma åt det som är paketsynligt.

   Om IDEA föreslog att göra en medlem paketsynlig hade du inte bytt till kursens inspektionsprofil.

   Package-visible ... varnar för detta.

4. Har du gjort alla implementationsdetaljer i en klass privata?

   Detta har vi diskuterat under föreläsningarna.

   Declaration access can be weaker signalerar när programkoden fortfarande skulle fungera även om specifika fält, metoder eller klasser blev "mer privata", till exempel för att de bara används från klassen där de deklareras. Detta antyder att det kanske handlar om en implementationsdetalj som borde vara privat.

Korrekt användning av typer

1. #undvikwrapper Används primitiva typer överallt där wrappers inte behövs?

   För att lagra ett heltal använder man till exempel normalt typen int, som är en primitiv typ som använder 4 bytes. I vissa situationer fungerar inte primitiva typer, till exempel när man vill ha en ArrayList av heltal: Arraylistor klarar bara att lagra (pekare till) objekt, och primitiva heltal är inte objekt i Java. Därför finns klassen Integer, vars objekt helt enkelt innehåller ett enda fält av typ int. Man "slår in" talet i ett objekt, och objektet kallas därför en wrapper.

   public class Integer
   {
       private int value;
       public Integer(int value) {
   	this.value = value;
       }
       ...
   }

   Wrappertyper som Integer och Boolean, med stor bokstav, är alltså användbara i vissa situationer – men i de flesta lägen tar de bara onödigt mycket tid och minne. Det finns mycket sällan anledning att använda dem som typ på fält, parametrar eller variabler, eller som returtyp för en metod. Primitiva typer är effektivare.

2. #elementtyp Anges elementtyp för alla generiska klasser?

   När du använder generiska typer som List, ange alltid elementtyp, t.ex. List<String>. Brott mot denna regel leder till sämre typsäkerhet.

   Raw use of parameterized class – Koden använder en generisk typ där ingen typparameter (elementtyp) är angiven.

   Unchecked warning / Unchecked call to '...' as a member of raw type '...' – Koden använder en medlem i en generisk typ där ingen typparameter är angiven. Till exempel kanske den använder add()-metoden i en lista som deklarerats som den råa typen List istället för List<String>. Detta leder till sämre typsäkerhet eftersom add() inte vet vilken elementtyp den returnerar. Skillnaden från föregående inspektion är att denna reporterar när man använder medlemmar i en variabel av rå typ, snarare än när man använder den råa typen själv.

3. Class explicitly extends a Collection class

   När man skapar en subklass till en Collection-klass (t.ex. class Foo extends List) öppnar man samtidigt upp för att andra kan manipulera den nya klassens objekt genom alla metoder som man ärver ner från Collection-klassen. Detta är ett typiskt fall när delegering brukar vara ett bättre val än ärvning – då bestämmer man själv vilken av Collection-klassens funktionalitet som ska exponeras utåt.

4. #collectioninterface Collection declared by class, not interface

   Man bör normalt deklarera collection-variabler eller fält med en gränssnittstyp, som List, istället för en konkret klass, som ArrayList. Detta hjälper till att undvika onödiga beroenden på exakt vilken konkret implementation av gränssnittet som används och gör det därmed lättare att byta implementation senare (till exempel att byta till LinkedList istället för ArrayList utan att byta ut deklarationstypen List). Med andra ord, använd "List list = new ArrayList()" istället för "ArrayList list = new ArrayList()", så ökar du din egen frihet att ändra koden i framtiden!

   Vissa undantag finns, där t.ex. specifika implementationer av List har ytterligare metoder som faktiskt behöver användas i en specifik implementation. Detta märker man om det inte går att kompilera koden när man byter typ till List. Då är det bra att dokumentera varför man faktiskt behöver använda t.ex. LinkedList som deklarationstyp.

   ConcreteCollectionDeclared –

5. Floating point equality comparison

   Koden testar om två floating point-värden är exakt lika. Detta är problematiskt eftersom värdena ofta är beräknade, och beräkningar sker med ändlig precision vilket ofta ger olika resultat beroende på i vilken ordning de utförs. Avrundningsfel är också vanliga. Exempel:

   float val = 0.1f + 0.1f;
   System.out.println(val == 0.2);
   

   Detta program ger svaret false!

   Om du har satt ett exakt värde, och sedan testar om variabeln har detta exakta värde, är jämförelsen inget problem. Exempel: Du sätter i vissa fall val=10, och testar sedan om val==10. I så fall kan du förklara detta vid varningen.

6. Integer multiplication or shift implicitly cast to long

   Ett värde beräknas som int (med 32-bitars heltal) och konverteras sedan till en long (med 64-bitars heltal). Ofta menade man att utföra även själva beräkningen med 64 bitar, vilket kräver att man cast:ar till rätt typ före operationen
   (long result = ((long)first) * second;). Som koden är skriven nu
   (long result = (long) (first * second);) kan den ge "overflow" utan att detta signaleras.

Felhantering och robusthet

1. #felmeddelanden Ges rimliga felmeddelanden vid felaktig inmatning från användaren?

   Ett program ska aldrig krascha på grund av att användaren har matat in "felaktig" information, vare sig det sker via inmatningsformulär eller via styrning av ett spel med tangentbord, mus eller liknande. Vi kan förutse att användaren kan göra i princip vad som helst, och behöver ta hand om det för att kunna ge rimliga felmeddelanden.

   Detta kan hanteras via exceptions eller via returvärden.

2. #filersaknas Agerar programmet korrekt om medskickade filer (bilder, ...) saknas?

   Många projekt använder sig till exempel av bilder som ska skickas med som en del av programmet. Om dessa bilder saknas kan man tänka sig att återhämta sig genom att istället t.ex. rita en enfärgad rektangel... men detta är inte ett krav. Istället kan det vara korrekt att signalera att filen saknas (ge ett tydligt felmeddelande med t.ex. System.out.println() eller en dialogruta) och sedan avsluta programmet med System.exit(felkod). Det är också OK om programmet helt enkelt kraschar på det ställe där filen skulle läsas in , till exempel genom att man gör new ImageIcon(...getSystemResource(...)...), där new ImageIcon(...getSystemResource(...)...) returnerar null. Då kan i alla fall programmeraren se var felet uppstod.

   Men det är inte korrekt att ignorera att filen saknas när man försöker läsa in en bild, så att (t.ex.) en bildpekare fortsätter vara null, så att programmet sedan kraschar på någon helt annan plats – till exempel i paintComponent()! Man ska inte heller avsluta programmet utan att signalera problem med felmeddelande eller exception – det blir förvirrande att programmet bara avslutas.

3. #stängresurser Stänger du alltid de resurser du öppnar?

   Java frigör automatiskt minne som man inte använder. längre, precis som Python. Men det finns andra resurser än minne som man kan behöva frigöra, till exempel öppnade filer, och det kan inte alltid göras automatiskt. Istället använder man en speciell form av try-satsen för att se till att resursen alltid frigörs – oavsett om koden faktiskt lyckas eller om den till exempel kraschar med en exception (som kanske fångas någon annanstans).

   Denna sats, som kallas try-with-resources, motsvarar with-satsen i Python. Den ser ut så här:

   static String readFirstLineFromFile(String path) throws IOException {
       try (BufferedReader br = new BufferedReader(new FileReader(path))) {
           return br.readLine();
       }
   }

   Nyckelordet try följs alltså av en variabeldeklaration och initialisering inom parenteser. Det objekt som skapas måste implementera AutoCloseable, som har en close()-metod som anropas automatiskt när man lämnar try-blocket – även om detta görs via en exception. Inuti blocket gör man allt det man behövde göra med resursen man allokerade.

AutoCloseable used without 'try'-with-resources – koden använder en sådan resurs utan att deklarera den på det vanliga sättet i en try-with-resources-sats. Detta kan ibland vara nödvändigt, om resursen ska "leva" längre än den metod där den skapas, men då måste man vara mycket noga med att se till att den alltid stängs på något annat sätt.

4. #fångalagombrett Fångar du "lagom breda" typer av fel?

   Om man t.ex. använder catch (Exception e) kommer alla typer av kontrollerade undantag (checked exceptions) att fångas upp i en och samma felhanterare. Det gäller även sådana som inte fanns när catch-satsen först skrevs, om koden inuti try ändras! Detta kan lätt leda till att man missar att introducera nya catch-satser för nya typer av fel. Fånga specifika feltyper istället! IDEA hjälper gärna till.

   Overly broad 'catch' block / Catch of '...' is too broad, masking exception(s) '...' – Koden fångar upp för breda grupper av undantag. Inspektionen upptäcker bara vissa specifika fall!

5. #kastalagombrett Kastar du lagom breda typer av fel?

   Det finns inget krav på att själv kasta exceptions, men om du gör det ska exceptionklassen vara väl anpassad till feltypen. Det går utmärkt att skapa egna exceptiontyper, till exempel HighscoreException.

   Prohibited exception thrown – Koden kastar ett undantag av generell typ, t.ex. Exception, istället för att använda en passande specifik typ. Inspektionen upptäcker bara vissa specifika fall!

6. #fångafelfel Fångar du bara sådana fel som ska fångas?

   Man ska inte fånga upp feltyper som normalt beror på felaktig programmering och som redan kan ha "förstört" något i resten av programmet. Detta kan till exempel gälla NullPointerException och IndexOutOfBoundsException. Istället för att fånga dessa ska man kontrollera i förväg om en pekare kan vara null, respektive kontrollera att indexet är korrekt innan man försöker indexera i en array eller lista.

   Samma gäller för IllegalArgumentException: Istället för att skicka in felaktiga argument och fånga IllegalArgumentException, är det bättre att kontrollera i förväg att argumentet är korrekt. (Och om man själv vill signalera ett problem som ska fångas upp, är det alltså normalt inte IllegalArgumentException man ska kasta för att göra den signaleringen. IllegalArgumentException ska kastas när någon har brutit mot metodens kontrakt genom att skicka in "uppenbart" felaktiga parametrar, t.ex. att en Timer ska anropa en metod var -20:e millisekund.)

   Prohibited exception caught – Koden försöker fånga upp en feltyp som normalt inte ska fångas. Inspektionen upptäcker bara vissa specifika fall!

   CatchUncheckedException –

7. #användfångat När undantag fångas, används det fångade objektet?

   Det är själva exception-objektet som fångas som vet mest om vilket fel som inträffade. Om man inte har extremt bra kunskap om exakt när felet kan inträffa behöver man fråga undantaget om vad som hände. Exempel:

   try {
       ...
   } catch (IOException e) {
       // Instead of logging the actual exception,
       // let us *assume* we know what happened... 
       System.out.println("Could not listen on port: "+port+".");
       System.exit(-1);
   }
   

   Här missar man att använda det undantag "e" som man fångar. Skriv ut felet med e.printStackTrace(), skicka vidare ursprungsfelet med throw efter att du har städat upp, eller (överkurs!) kasta en "chained exception".

   I undantagsfall (!) kan det vara rimligt att inte använda undantaget. I så fall kan man döpa variabeln till "ignore" istället för "e", så vet IDEA att man medvetet ignorerade den.

   Unused 'catch' parameter – Koden fångar ett undantagsobjekt men använder det inte.

8. #stacktrace När du fångar ett undantag som inte kan "fixas", skriver du ut (eller loggar) en stacktrace?

   Som vi har diskuterat under föreläsningarna är det väldigt bra för avlusning (debugging) att veta både var fel uppstår och vem som då hade anropat vem. Då behöver man se till att anropskedjan till exempel skrivs ut (som en stack trace) med ex.printStackTrace().

   Detta är så klart inte det enda som behöver göras: Man behöver också se till att felet hanteras eller att (t.ex.) användaren meddelas att något gick fel. Loggar räknas inte som meddelanden till användaren.

   CatchPrintWithoutTrace –

9. #hanteracatch Hanterar programmet fångade exceptions (undantag) på rätt sätt?

   Om du väl har fångat ett fel, låt då inte programmet bara gå vidare. Föreläsningsexempel:

   public class WordProcessor {
       private Document doc = null;
   
       public WordProcessor(String filename) {
           try {
               loadFile(filename);
           } catch (FileNotFoundException e) {
               System.out.println("File not found");
           }
          System.out.println("Size is " + doc.getLength());
       }
   
       private void loadFile(String filename) throws FileNotFoundException {
           FileInputStream is = new FileInputStream(filename);
           // ...
           doc = parseDocumentFrom(is);
       }
   }

   I detta exempel: Om filen saknas skriver konstruktorn ut "File not found", men fortsätter sedan vidare och får NullPointerException i anropet till doc.getLength().

   Detta innebär även att main() inte ska skicka exceptions vidare till "systemet", utan fånga och ta hand om dem.

   Empty 'catch' block – upptäcker fallet när catch-satsen inte gör något alls. Däremot upptäcks inte att man t.ex. skriver ut ett meddelande och går vidare.

   CatchMissingInMain –

Designmönster

Det finns inget krav på att designmönster ska användas – men om de används, ska de användas på korrekt sätt.

1. Om du använder State, använder du det korrekt?

   Meningen med State-mönstret är att man ska kunna byta ut beteende hos ett objekt beroende på det tillstånd det är i. Med beteende menas här programkod – ett objekt har tillstånd (data) och beteende (metoder som kan anropas).

   Det räcker då till exempel inte att ett State-objekt har en metod som returnerar en konstant. Man måste ha en eller flera metoder som kan anropas i State-objektet och som faktiskt gör något kvalitativt annorlunda beroende på vilken typ av State-objekt man har just nu.

   Om man bara behöver hålla reda på "vilket av de 5 tillstånden jag är i just nu", så att annan kod (utanför State-klasserna) kan agera olika beroende på tillstånd, räcker det utmärkt att använda med enum med 5 olika värden. Att använda det fullständiga State-mönstret i detta läge blir då "overkill".

2. Om du använder Singleton, är det nödvändigt?

   Detta har diskuterats tidigare på sidan.

Körbara program

Projektet ska resultera i ett färdigt program som kan "levereras" för enkel installation och körning på någon annans dator – till exempel hos handledaren som ska testa inlämningen. Detta ställer ytterligare krav på programmet.

1. #bifogabibliotek Skickar du med eventuella tredjepartsbibliotek?

   Om du använder annan kod än den som finns medlevererad i Java 8 måste denna kod finnas med i inlämningen (t.ex. i ditt Git-repo), och måste vara konfigurerad i IDEA så att handledaren enkelt kan kompilera och starta programmet.

   Inga extrasteg ska krävas (nedladdning, separat installation, ...)!

2. #användresurser Läses programmets egna filer in via resurser?

   När ett program läser in egna medskickade filer, t.ex. bilder, spelbanor eller andra datafiler, måste programmet först hitta filerna. Men programmet kan vara installerat var som helst, och kan till och med vara ihoppackat i en enda fil, ett JAR-arkiv. Därför ska man inte använda vanlig filhantering för sådana filer, utan Javas inbyggda stöd för "resurser". Då hittas filerna automatiskt på samma ställe som själva programmet. Vi har diskuterat detta under GUI-föreläsningen och använder då ClassLoader.getSystemResource().

   Notera att filer som användaren skapar varken kan eller bör hanteras på detta sätt. Där är det användaren som bestämmer var de ska ligga, och användaren som måste ange korrekt sökväg (på kommandoraden, i en konfigurationsfil, via fildialog, eller liknande).

   Samma gäller filer som programmet skapar under körningen. Dessa kan inte sparas som en del av programmet (i JAR-arkivet) utan sparas på annan lämplig plats, t.ex. i nuvarande katalog.

   HomeOrSrcDirectory upptäcker vissa fall där man refererar till en hemkatalog eller en src-katalog. Detta är korrekt i vissa fall, men inte för att läsa filer som skickas med projektet. Då bör man använda resurshantering istället.

   FileFromResource upptäcker vissa fall där man hämtar in en resurs-URL men sedan skapar ett File-objekt från den. Detta antyder att man försöker läsa in en fil via vanlig filhantering, vilket inte fungerar om resursen ligger packad i en JAR-fil.

   ImageIconFromFile upptäcker vissa fall där man skapar en ImageIcon från ett filnamn. Här bör man använda resurshantering istället.

   ImageFromFile upptäcker vissa fall där man använder ImageIO tillsammans med ett filnamn. Här bör man använda resurshantering istället.

   PathFromResource upptäcker vissa fall där man plockar ut en sökväg från en resurs-URL. Detta kan bero på att man sedan vill använda sökvägen för att öppna en fil, vilket inte täcker fallet där resursen t.ex. pekar ut en fil inuti ett JAR-arkiv.

   FileSeparatorInGetResource upptäcker vissa fall där man försöker använda File.separator inuti ett anrop till getResource(). Detta är fel: File.separator är olika på olika plattformar, men getResource() använder en URL, där separatorn alltid är /.

Projektrapporten

Inga kommentarer för tillfället. Följ instruktionerna i mallen!

Inspektion: Vanliga misstag och fällor

Dessa inspektioner varnar för kodmönster som ofta indikerar rena misstag i programmet. Vissa av dem handlar om "fällor" där språket eller klassbiblioteket fungerar på ett annat sätt än man annars kunde ha trott.

1. Call to overridden method '...' during object construction

   En konstruktor för klass A anropar en metod som finns implementerad i klass A , men är omimplementerad (overridden) i en underklass B. Detta gör att klass A inte har full kontroll över vad som sker när den konstrueras (det är ju delvis kod i klass B som utförs), vilket kan leda till problem.

2. Comparable implemented but 'equals()' not overridden / Class '...' implements 'java.lang.Comparable' but does not override 'equals()'

   Gränssnittet Comparable innehåller en compareTo()-metod som jämför om ett objekt är före, efter, eller på samma plats som ett annat i en viss sorteringsordning. Om man implementerar detta måste man också implementera en egen version av equals(), som stämmer överens med compareTo(): Enligt kontraktet gäller att x.equals(y) om och endast om x.compareTo(y) == 0.

   Om detta inte uppfylls kan datatyper som använder både compareTo() och equals() för jämförelse bli mycket förvirrade.

3. Condition '...' is always false / Condition '...' is always true

   IDEA genomför en dataflödesanalys och identifierar villkor vars värden är kända redan vid kompileringstillfället. Detta uppstår ofta på grund av tankefel i programmeringen och är därför viktigt att åtgärda: Om villkorets värde är känt, kanske du egentligen har skrivit fel i själva villkoret!

4. Duplicate condition in 'if' statement

   En if-sats har flera grenar som testar samma villkor, eller testar samma villkor flera gånger i en gren. Detta är sällan vad programmeraren menade, och kan bero på att man har missat något och skrivit fel – kanske man ska ta bort en gren, eller ville ha ett annat villkor på den grenen.

5. Duplicate condition on '&&' or '||'

   Ett villkorsuttryck testar samma villkor flera gånger, till exempel foo && bar && foo. Detta är sällan vad programmeraren menade, och kan indikera att man antingen ska ta bort en del av uttrycket eller ville ha ett annat villkor den andra gången.

6. #HELTALSDIVISION Integer division in floating-point context

   Här använder du till exempel ett uttryck såsom Math.sqrt(a / b), där både a och b är heltal, till exempel int a=174 och int b=10.

   • Matematiskt har vi ju a/b = 17.4, men eftersom a och b är av heltalstyp utförs en ren heltalsdivision. Detta returnerar i sin tur också ett heltal, till exempel 17.
   • I nästa steg vill man skicka in detta till Math.sqrt(), som bara tar flyttal. Vid anropet måste 17 först konverteras till flyttalet 17.0.
   • Sedan utförs själva sqrt()-operationen, som returnerar cirka 4.123
   • Men om vi hade börjat med flyttalsdivisionen 174.0/10.0 hade vi fått Math.sqrt(17.4) vilket är cirka 4.171. Antagligen var det egentligen detta vi ville – varför skulle vi vilja slänga bort precision i divisionen genom att först göra oss av med alla decimaler innan vi drar roten ur resultatet? Det vi troligen ville göra kan vi uppnå genom att konvertera minst ett av heltalen till flyttal först: Math.sqrt(a / (double) b), till exempel.

   Vi tar ytterligare ett exempel där man använder ett uttryck såsom (int) Math.floor(a / b), där både a och b är heltal.

   • Eftersom a och b är heltal utförs en ren heltalsdivision, som i sin tur också returnerar ett heltal, till exempel 17.
   • I nästa steg vill man skicka in detta till floor(), som bara tar flyttal. Vid anropet måste 17 först konverteras till flyttalet 17.0.
   • Sedan utförs själva floor()-operationen, som ser till att eventuella decimaler nollas. Decimalerna var redan noll (eftersom det ursprungliga talet var ett heltal), så 17.0 blir till 17.0 igen, utan förändring.
   • Sedan ska detta returnerade flyttal cast:as till int och konverteras då tillbaka från 17.0 till talet 17, som man redan hade.
   • Alltså hade man i just detta fall fått exakt samma resultat genom att använda a/b utan ytterligare operationer.
   • Alternativt hade man även här kunnat använda Math.floor(a / (double) b), men detta hade då resulterat i Math.floor(17.4). Då gör Math.floor() samma sak som heltalsdivisionen ändå hade gjort: Klipper av decimalerna, så svaret blir fortfarande just 17.0 – onödigt ineffektivt att gå omvägen via flyttal.

7. Method invocation '...' may produce 'java.lang.NullPointerException'

   IDEA har analyserat koden och kommit fram till att du försöker anropa en metod i ett objekt med hjälp av en pekare som kan vara null (inte peka på något objekt). Exempel: objekt.metod() där objekt == null.

   I vissa fall beror detta på att du har glömt testa om objekt == null. I andra fall är det större strukturella fel. Det finns också fall där IDEA varnar trots att pekaren faktiskt inte kan vara null (av anledningar som flödesanalysen missar).

8. Object comparison using '==' instead of 'equals()'

   Objektjämförelse med '==' testar om två pekare pekar på exakt samma objekt, på samma maskinadress. Nästan alltid vill man istället använda 'equals()', som ska jämföra om två objekt är lika (om två strängar innehåller samma tecken, till exempel).

   Vissa undantag finns. I implementationen av equals() eller compareTo() kan man av effektivitetsskäl börja med att testa om man jämför ett objekt med sig själv (samma pekare), vilket kräver att man använder '=='. Det finns också andra fall där det faktiskt är identitet, och inte likhet, som man vill testa. De flesta fall av "==" mellan objekt som vi ser i inlämnade projekt är dock misstag.

9. Result of method call ignored

   IDEA har en lista på metoder vars returvärden är viktiga och inte får ignoreras. Det inkluderar t.ex. File.mkdir(), som indikerar om metoden lyckades via returvärde istället för via undantagshantering, och Object.hashCode(), som inte alls har sidoeffekter utan anropas enbart för dess returvärde.

   Metoden File.createNewFile() är till för att atomärt skapa en fil om den inte finns (och ingen annan hann före); se dokumentationen. Ignorerar man returvärdet har man inte använt metoden på det sätt den är tänkt att användas. Man behöver inte använda denna för att skapa en ny fil att skriva till.

10. Result of object allocation ignored / Result of '...' is ignored

    Indikerar att man har skapat ett objekt men inte sparat en pekare till objektet någonstans. I de allra flesta fall är detta meningslöst och leder bara till att objektets kommer att samlas in som skräp.

    Vissa undantag finns i Javas klassbibliotek, främst i fråga om Frame / JFrame där konstruktorn skapar ett fönster som registrerar sig i fönstersystemet och sedan "lever vidare" på egen hand. Inspektionen får alltså ignoreras för Frame och dess subklasser. Denna metod där objekt registrerar sig själva någonstans är normalt inget man ska efterlikna, annat än i mycket specifika undantagsfall.

11. Suspicious indentation after control statement without braces

    Rapporterar när indenteringsnivån är missvisande. Kan fixas genom att filen indenteras om (Code|Auto-Indent Lines) eller formatteras om (Code|Reformat Code).

12. Unsafe lazy initialization of 'static' field /
    Lazy initialization of 'static' field ... is not thread-safe

    En statisk variabel tilldelas sitt initialvärde av en metod som skulle kunna anropas från flera trådar på samma gång, och det finns ingen trådsynkronisering som garanterar att variabeln bara initialiseras en enda gång. Ofta kan man istället undvika "lazy initialization" och helt enkelt initialisera variabeln där den deklareras. Då kan detta problem inte uppstå. Vill man absolut använda lat initialisering (där variabelns värde inte beräknas förrän det verkligen ska användas) kan man t.ex. se Initialization-on-demand holder idiom, som är trådsäkert.