DOC

Checklist Code Review Java

By Evelyn Freeman,2014-04-24 14:25
9 views 0
Checklist Code Review Java

Appendix

    Code review Mortgage prototype

Main.java

    MsgBox.java

    MSGContentChecker.java

    MSGDateChecker.java

    MSGEntry.java

    MSGEntryException.java

    MSGEntryForm.java

    MSGEntryList.java

    MSGEntryListException.java

    MSGExpencesDialog.java

    MSGField.java

    MSGFieldException.java

    MSGInvestmentEntry.java

    MSGInvestmentList.java

    MSGMortgageEntry.java

    MSGMortgageList.java

    MSGNumberChecker.java

    YesNoDialog.java

Checklist Code Review

Organisatie

    ? Elke klasse in aparte file Gerelateerde klassen in aparte package ? alle klassen zitten in 1 package. Niet

    geordend naar gerelateerde klassen

    ? Adequate inheritance structuur

    Afhankelijkheden van klassen expliciet ?

Naamgeving

    ? Betekenisvolle namen

    ? Samengestelde namen: deel begint met

    hoofdletter

    ? Packagenaam volgens standaard

    ? Klassennaam begint met hoofdletter /

    zelfstandig naamwoord

    ? Methodenaam begint met kleine letter /

    werkwoord

    ? Constantennaam met hoofdletters

    ? Get- en setmethoden consequent

    benoemd

    ? Consequent EN of NL

Layout

    Consequente indentatie ? Indentatie wordt niet altijd correct

    toegepast. Geen consequent aantal tabs na

    een methodedeclaratie

    Fout in: YesNoDialog.java

     Main

    Consequente plaatsing haakjes ? Soms worden openingshaakjes niet op

    de volgende regel geplaatst terwijl dat in

    de rest van de code wel gebeurt.

    Fout in: MSGEntryException

     MSGEntryListException

     MSGEntryList: methode getEntry

    ? Groepering methoden en

    instantievariabelen

    Regellengte < 80 ? Maximale regellengte wordt vaak

    overschreden.

    Fout in: MSGContentChecker.java

     MSGEntryForm.java

     MSGMortgageEntry.java

     MSGMortgageList.java

     MSGNumberChecker.java

     Main.java

     MSGDateChecker.java

     MSGEntryException.java

     MSGEntryListException.java

     MSGEntryList.java

     MSGEntry.java ElemType [] ArrayName (i.p.v. n.v.t.

    ElemType ArrayName[])

    Methode body < 40 LOC ? Sommige methoden overschrijden de

    limiet van 40 regels

    Fout in: MSGEntryForm: resetContents,

     doNext,

     doStore,

     GetIsUnsaved

     Main.java : doLoad,

     doSave

Commentaar

    Adequaatie/niet-triviaal ? soms een beetje triviaal zoals bij

    setVisible(true) als commentaar show the

    window

    Beschrijving klassen (o.a. doel, auteur, ? Datum en versie ontbreken in alle

    datum, versie) klassen

    ? Beschrijving instantie variabelen

    Beschrijving methoden: effect / ?, pre- en postcondities worden niet

    parameters / resultaat / pre-/postcondities weergegeven in alle klassen

    ? Beschrijving algoritmen

    Beschrijving locale variabelen ? Locale variabelen soms niet

    beschreven.

    Fout in: Main.java : doSave

     MSGDateChecker.java

    ? Gebruik JavaDoc

Semantiek

    ? Klassenhierarchie conceptueel (geen

    implementatie inheritance)

    ? Coherente methoden (afgebakende

    functionaliteit)

    ? Juist gebruik modifiers (o.a. alleen

    private instantievariabelen)

    ? Initialiseren instantievariabelen (zowel

    basistypen als objecttypen)

    ? Afhandeling van excepties met catch

    ? Gebruik van equals bij vegelijking

    objecten

    Een main voor elke klasse met unit tests n.v.t aangezien er geen unit test

    voorkomen

    ? Close file na write

Evaluatie

De java code van het mortgage prototype voldoet qua naamgeving en semantiek aan

    de vooraf gestelde voorwaarden. Bij de onderdelen Organisatie, Layout en

    Commentaar valt echter nog wel wat te verbeteren.

    De organisatie kan verbeterd worden door gerelateerde klassen in aparte packages op

    te nemen. Zoals alles van entrylist in 1 package. De klassen zitten nu allemaal in 1

    package terwijl ze niet echt aan elkaar gerelateerd zijn.

    Bij de layout moet meer gelet worden op het consequent indenteren en het consequent

    plaatsen van haakjes. Dit wordt nu nog niet altijd gedaan. Het is zinvol om dit wel te

    doen zodat de code beter leesbaar wordt. Ook wordt de regellengte vaak overschreden

    en ook het maximum aantal regels van een methode. Dit komt ook niet ten goede aan

    de leesbaarheid.

    Op het commentaar zijn ook enkele aanmerkingen te maken. Soms is het commentaar

    triviaal zoals bij setVisible(true) als commentaar show the window vermelden terwijl

    setVisible eigenlijk al genoeg zegt. Ook wordt bij het begin van de klasse niet de

    datum en versie vermeld. Het is wel zinvol om dit te doen zodat je meteen kan zien

    met welke versie je te maken hebt, dit voorkomt eventuele misverstanden. Verder

    worden bij de methoden niet de pre en post condities weergegeven. Dit is wel handig

    zodat als iemand anders met het project verder moet hij precies weet waar hij op moet

    letten. Tot slot worden sommige locale variabelen niet becommentarieerd.

Metrics

Methode doLoad() van klasse Main

private void doLoad() { boolean ready = false; 1 File dataFile = null; // Present the load-dialog until a file is chosen that exists. while ( !ready ) { FileDialog loadDialog = new FileDialog ( this, "Load data", FileDialog.LOAD ); // Initialize the file and directory. 2/3 if ( curdir != null ) loadDialog.setDirectory ( curdir ); 4/5 if ( curfile != null ) loadDialog.setFile ( curfile ); loadDialog.setVisible(true); // Remember the file and directory. curdir = loadDialog.getDirectory(); 6 // If a file is chosen: if ( loadDialog.getFile() != null ) { curfile = loadDialog.getFile(); dataFile = new File ( curdir, curfile ); // If the file exists, proceed. 8 if ( dataFile.exists() ) { 9 ready = true; } } 7 else { // The cancel button was pressed. return; } 10 } // The FileInputStream reads raw data from a file, the ObjectInputStream reads // objects from the FileInputStream. FileInputStream fis = null; ObjectInputStream ois = null; 12 try { fis = new FileInputStream ( dataFile ); ois = new ObjectInputStream ( fis ); mortgageList = (MSGMortgageList)ois.readObject(); investmentList = (MSGInvestmentList)ois.readObject(); expenses = ois.readDouble(); } catch ( StreamCorruptedException e ) 13/14 { if ( debug ) System.err.println(e.toString()); 15 MsgBox msgbox = new MsgBox ( this, "Error!", "Not a datafile!" ); } catch ( InvalidClassException e ) 13/14 { if ( debug ) System.err.println(e.toString()); MsgBox msgbox = new MsgBox ( this, "Error!", "Incompatible 15 datafile!" ); } catch ( IOException e ) 13/14 { if ( debug ) System.err.println(e.toString()); 15 MsgBox msgbox = new MsgBox ( this, "Error!", "Load failed!" ); } 13/14 catch ( ClassNotFoundException e ) { if ( debug ) System.err.println(e.toString()); 15 MsgBox msgbox = new MsgBox ( this, "Error!", "Load failed!" ); }

     finally 16 { // This code is executed ALLWAYS. If files were opened, close them. if ( ois != null ) { 17 try { ois.close(); } 18 catch ( IOException e ) { } } 19 if ( fis != null ) { 20 try { fis.close(); } 21 catch ( IOException e ) { } } } 22 // Of course the funds must be recomputed. 11 recompute(); }

    12123

    13

    45

    14

    615

    16

    8717

    109

    18

    11

    2019

    21

    22

Methode getNetPayments() van klasse MSGMortgageList.java

1/ public double getNetPayments () 2 { if ( entryList.size() == 0 ) return 0; double price=0, balance=0, premium=0, tax=0, income=0, temp1=0, temp2=0, 3 sum1=0, sum2=0; for ( int i = 0; i < entryList.size(); i++ ) 4 { MSGEntry curEntry = (MSGEntry)(entryList.elementAt(i)); 5 try { price = (Double.valueOf ( (String)curEntry.getField(MSGMortgageEntry.F_PRICE).getContents() )).doubleValue(); balance = (Double.valueOf 6 ( (String)curEntry.getField(MSGMortgageEntry.F_BALANCE).getContents() )).doubleValue(); premium = (Double.valueOf ( (String)curEntry.getField(MSGMortgageEntry.F_PREMIUM).getContents() )).doubleValue(); tax = (Double.valueOf ( (String)curEntry.getField(MSGMortgageEntry.F_TAX).getContents() )).doubleValue(); income = (Double.valueOf ( (String)curEntry.getField(MSGMortgageEntry.F_INCOME).getContents() )).doubleValue(); } catch ( NumberFormatException e ) { // Indeed, because we use the field names from MSGMortgageEntry. System.err.println( "NumberFormatException occurred: " + e.toString() ); System.err.println( " (THIS CANNOT OCCUR!)" ); } catch ( MSGEntryException e ) 7 { // Indeed, because we use the field names from MSGMortgageEntry. System.err.println( "MSGEntryException occurred: " + e.toString() ); System.err.println( " (THIS CANNOT OCCUR!)" ); } temp1 = ( price / NUM_MORTGAGE_PAYMENTS ) + ( ( balance * INTEREST_RATE ) / 52.0 ) + ( ( tax + premium ) / 52.0 ); temp2 = temp1 - ( income * MAX_PERCENT_OF_INCOME ); 8 sum1 += temp1; sum2 += temp2; } return (sum1 - sum2); } 9 10

    312

    410

    5

    6

    78

    9

Methode getReturnOnInvestments() van klasse MSGInvestmentlist.java

1/ 2 public double getReturnOnInvestments () { if ( entryList.size() == 0 ) return 0; 3 double annret=0, sum=0; 4 for ( int i = 0; i < entryList.size(); i++ ) { try { 6 annret = ( Double.valueOf ( (String)((MSGEntry)(entryList.elementAt(i))).getField(MSGInvestmentEntry.F_RETURN ).getContents() )).doubleValue(); } catch ( NumberFormatException e ) { 7 // Indeed, because we use the field names from MSGInvestmentEntry. System.err.println( "NumberFormatException occurred: " + e.toString() ); System.err.println( " (THIS CANNOT OCCUR!)" ); } catch ( MSGEntryException e ) { 8 // Indeed, because we use the field names from MSGInvestmentEntry. System.err.println( "MSGEntryException occurred: " + e.toString() ); System.err.println( " (THIS CANNOT OCCUR!)" ); } sum += annret; 9 } return ( sum / 52.0 ); 5 }

    312

    45

    6

    78

    9

Evaluatie metrics

De complexiteit van de drie behandelde methoden wordt berekend aan de hand van de

    volgende formule: Aantal edges - Aantal nodes + 2. De uitkomst geeft de complexiteit

    weer van de geteste methode. Een uitleg van deze getallen is te vinden in het raport.

    Dit geeft het volgende resultaat: doLoad() = 32 22 + 2 = 12

    getNetPayments() = 12 10 + 2 = 4

    getReturnOnInvestments() = 11 9 + 2 = 4

Uit deze complexiteitgetallen valt af te lijden dat de broncode grotendeels goed te

    testen is, aangezien de complexiteit van 2 van de 3 geteste methoden onder de grens

    van 10 blijft. Alleen voor de methode doLoad() en waarschijnlijk ook doSave() is dit

    niet het geval. Deze methoden komen boven de grens van 10 uit en zijn hierdoor niet

    goed te testen.

    Over het algemeen valt dus te zeggen dat de code goed te testen en te onderhouden is

    afgezien van een klein aantal methoden.

Report this document

For any questions or suggestions please email
cust-service@docsford.com