Archive for the ‘Kodestil’ Category

Tabt i detaljerne

Tuesday, September 7th, 2010

Sådan vasker Microsoft hænderHosstående opslag hænger på herretoilettet hos Microsoft i Hellerup. Det er en detaljeret, illustreret gennemgang af, hvordan man vasker hænder. Hvis vi ser bort fra den reelle risiko for en TL;DR-reaktion, må den minutiøse beskrivelse være tilstrækkelig til at tilgodese selv den mest nidkære bureaukrats behov for information, og ingen skulle således være i tvivl om, hvordan man vasker hænder i Hellerup.

Opslaget er mere underholdende end gavnligt i mine øjne, men det er desværre også et billede på den kode, de fleste af os til tider skriver. Jeg har læst og skrevet masser af kode, der i detaljeringniveau kan måle sig med ovenstående. Jo tættere vi kommer på metallet, desto tyndere bliver vores abstraktioner, og dermed bliver vi nødt til at redegøre for flere detaljer.

Vi kan således ikke helt undgå denne problematik, men jo mere vi kan styre uden om den slags desto bedre.

Problemet er, at det kan være svært at se, hvad målet er. Vi har specificeret, hvad der skal gøres, men det kan være svært at se, hvad det egentlig er, vi gerne vil opnå, og dermed kan det være svært at sige noget konkret om implementeringen. Fungerer den efter hensigten? Er der bivirkninger? Kan den optimeres om nødvendig? Kan den paralleliseres?

Hvad er formålet med instruktionerne i opslaget? Eller sagt på en anden måde: Hvad er det ønskede slutresultat, hvis vi følger proceduren? Er det rene hænder, reduktion af sæbebeholdning eller en sindrig form for håndaerobics i vand? Er det, vi ønsker at opnå, blot en delmængde af det samlede resultat? Hvordan skelner vi i så fald mellem de nødvendige og overflødige trin i processen for at opnå det ønskede?

Det kan være svært at adressere disse spørgsmål, da svarene ligger gemt i detaljerne.

Lad os se på et eksempel mere. Betragt følgende kode.

var numbers1 = new[] { 2, 4, 6, 7, 8, 9 };
var numbers2 = new[] { 1, 2, 3, 4, 5, 6, 9 };
var numbers3 = new[] { 1, 5, 7, 9 };

var result = new List<int>();

foreach (var n1 in numbers1) {
   foreach (var n2 in numbers2) {
      if (n1 == n2) {
         bool found = false;
         foreach (var n3 in numbers3) {
            found |= n2 == n3;
         }
         if (!found) {
            result.Add(n2);
         }
         break;
      }
   }
}

Hvad laver ovenstående? Med mindre man har set konstruktionen et utal af gange, skal de fleste af os nok lige tænke en gang eller to for at gennemskue, hvad formålet med koden er. Problemet er, at der er mange detaljer, så vi er næsten tvunget til at løbe et lille eksempel igennem, før vi kan gennemskue resultatet af koden. Det er ikke et stort problem, men vi skal bruge flere ressourcer på at forstå koden, end vi behøver.

Der er andre nærliggende spørgsmål, der ligeledes kan være svære at svare på her: Kan vi optimere koden, hvis behovet opstår? Hvor mange uafhængige operationer er der i koden? Kan disse paralleliseres?

Som du sikkert har gennemskuet, er indholdet af result alle de elementer, der er i både numbers1 og numbers2, men ikke i numbers3.

Det lugter lidt af mængdeoperationer, og med den erkendelse kan vi pludselig tale om problemet i domænespecifikke termer. Vi kan tilmed omskrive koden til at bruge .NETs mængdeoperationer. Ovenstående kan således udtrykkes som:

result = numbers1.Intersect(numbers2).Except(numbers3).ToList();

Vi kan selvfølgelig glæde os over, at dette er meget kortere, men det er kun en af gevinsterne her. Ved at bruge domænespecifikke abstraktioner kan vi kommunikere i et sprog, der giver mening i forhold til problemet, og vi er fri for at bekymre os om, hvad der skal til for at opnå det ønskede resultat. Vores kode er således mere deklarativ. Vi specificerer, hvad vi vil have, fremfor hvordan vi vil finde frem til resultatet i et sprog, der relaterer til vores problemdomæne.

Vi kan også let identificere antallet af nødvendige operationer, og derved har vi bedre mulighed for at identificere overflødige trin. I og med at vi arbejder med abstraktioner, er det derimod svært at sige noget om hvorvidt disse er implementeret optimalt, men med mindre vores målinger viser, at koden ikke kører optimalt, er vi sikkert bedre tjent med at bruge kode, der er testet og dokumenteret fremfor vores egen hjemmebryggede kode.

Hvis vi forestiller os, at result i stedet repræsenterer listen af kunder, der skal have et specielt tilbud. Således kunne numbers1 repræsentere de kunder, der interesserer sig for hi-fi, numbers2 kunne være listen af kunder, der interesserer sig for vvs-artikler, og numbers3 kunne holde styr på de kunder, der ikke har betalt til tiden. I så fald kunne vi bruge vores mængdeoperationer til at sende brev til relevante kunder omkring et nyt spabad med surround sound (hvis det ikke findes, skal det nok komme).

Koden ville essentielt være den samme, men den nuværende navngivning er helt i skoven. Koden taler om tal og mængder, og vi har således ingen forbindelse mellem koden og domænet. Så selvom .NET tilbyder os de nødvendige operationer, ville vi med fordel kunne indkapsle mængdeoperationerne i en eller flere metoder, hvis navne kan bygge bro til domænet.

Så hvad er lektien her?

Undgå udpenslende kode. Se om problemet kan udtrykkes på en måde, så vi kan bruge eksisterende funktionalitet. Hvis det ikke er tilfældet, så skab de nødvendige abstraktioner. I begge tilfælde bør abstraktioner navngives (evt. indpakkes) i konstruktioner, der giver mening i problemdomænet.

Følger vi disse trin, får vi kode, der er lettere at læse. Vi får muligvis mere kode, men hver del bliver kort, præcis og formuleret i et sprog, der passer til problemet. Vi får lettere ved at ræsonnere om kodens egenskaber, da den er lettere at overskue. Kode, der er let at forstå, er også lettere at vedligeholde og udvide.

Abstraktioner som typer og metoder er blandt af de vigtigste værktøjer i vores værktøjskasse, fordi de giver os mulighed for at opsplitte kompleksitet i overkommelige størrelser og navngive disse i et sprog, der afspejler vores problemdomæne. Brug dem!

Exception eller ej

Monday, February 15th, 2010

Mark Seemann stillede for nylig et spørgsmål på Twitter, der lød noget i retning af ”Skal Delete(int id) kaste en exception hvis det element, id repræsenterer, ikke eksisterer, eller skal den bare undlade at gøre noget?”.

Ræsonnementet for ikke at gøre noget er naturligvis, at hvis vi beder om at få slettet noget, og det ikke er der, så er sluttilstanden jo den ønskede, uanset om vi slettede noget eller ej.

Mit svar var, at med den signatur skulle metoden kaste en exception, og i dette indlæg vil jeg forsøge at uddybe lidt. Der er to grunde til, at jeg synes, en exception er på sin plads i dette tilfælde.

For det første er vi vant til, at Delete() på framework-typer som f.eks. File, Directory og DataRow alle smider passende exceptions, hvis de kaldes med et element, der ikke findes. Så hvis vi vil holde os til Principle of least surprise, er en exception det rette valg.

For det andet, og måske vigtigere, vil det være umuligt for den kaldende kode at afgøre, hvorvidt operationen var en succes eller ej, hvis Delete() ignorerer eventuelle fejl. Da vi ikke kan sige noget om, hvilken kontekst Delete() vil blive kaldt i, er det ikke rimeligt, at tage beslutninger på den kaldende kodes vegne. Vi ved med andre ord ikke, om koden har brug for at skelne mellem succes og fejl.

Hvis koden kalder Delete(), må vi antage, at det er fordi applikationens tilstand af en eller anden grund får det til at give mening. Koden antager altså, at det kan lade sig gøre. Ved at ignorere en eventuel fejlsituation, fratager vi den kaldende kode muligheden for at opdage, at denne antagelse ikke holdt stik. Vi risikerer altså at skjule en fejl i koden.

Mark fulgte desuden op med følgende spørgsmål: Gør det nogen forskel om Delete() kaldes i et miljø med flere tråde? Nej, ikke i min optik.

File.Delete() er i høj grad underlagt denne problemstilling. Uanset at vi kan undersøge, om en given fil eksisterer lige inden, vi kalder Delete(), er denne operation altid underlagt en race condition. Filen kan forsvinde mellem vores kald til Exists() og Delete(), og den eneste måde, vi kan opdage, om dette er tilfældet, er, hvis Delete(), kaster en exception. Derefter må det være op til den kaldende kode, om den vil reagere på dette eller ej. Ved at undlade at reportere fejlen, fratager vi den kaldende kode muligheden for at reagere.

Derfor synes jeg, en exception er det rette valg i denne situation.

Kodekommentarer er ofte spild af tid

Wednesday, February 10th, 2010

Jeg har ikke tal på hvor mange gange, jeg har hørt udviklere efterlyse flere kommentarer i koden, men uden at have lavet en egentlig undersøgelse, er min fornemmelse, at jeg har set langt flere dårlige end gode kommentarer i min tid som udvikler. Derfor er det ikke just nærliggende for mig at efterspørge flere kommentarer. Nærmest tværtimod.

Lad mig dog slå fast, at jeg bestemt anerkender værdien af kommentarer i de tilfælde, hvor de faktisk tilfører koden værdi. I alle de andre tilfælde gør de ofte mere skade end gavn. Lad os se lidt på, hvad der adskiller de brugbare kommentarer fra de ubrugelige.

Eksempler

Problemerne med kommentarer er mange, så lad os se på nogle af dem. Min liste er næppe udtømmende (lad os endelig få flere skrækeksempler), men blot et udsnit af de brugsmønstre, vi sikkert alle er stødt på fra tid til anden. Det første eksempel er banalt og synes ikke at gøre den store skade:


// reset counter
cnt = 0;

Men lad os ikke lægge ud med at tale om skade. Lad os i stedet spørge: Hvilken værdi tilfører denne kommentar koden? Ingen. Den fortæller mig godt nok, at udvikleren har været for doven eller måske ikke har evnet at omdøbe sin variabel til Counter eller noget mere sigende. At forklare at værdien bliver nulstillet, når den sættes til nul, er en lige så overflødig oplysning som at tvillinger kommer i par. Kommentaren giver os altså ikke mere information end vi snildt kunne have udtrykt i selve koden, så undgå gentagelsen. DRY gælder også for kommentarer.

Det største problem ved denne slags kommentarer er desværre, at de ofte kommer i hobetal. Hvis udvikleren af en eller anden grund tror, at de faktisk er til gavn, bliver den slags ligegyldigheder strøet ud over det hele. Det gør koden længere uden at gøre noget som helst positivt for læsbarheden. Det er spild af alles tid.

En afart af ovenstående er, når kommentarer bruges til at forklare værdier. Det kan være ved tildeling som ovenfor eller ved kald af metoder. Eksempel:


// payout occurs every 3 weeks
Payout(3);

Brug dog en konstant i stedet for en kommentar, så er der tilmed chance for, at den kan bruges igen.

Lærebog i C#

Lad være med at skrive kommentarer, der forklarer, hvad forskellige sprogkonstruktioner gør. Antag at læseren kender sproget. Skulle det ikke være tilfældet, er kommentarer alligevel ikke løsningen på det problem.

Beskrivelser af forskellige sprogkonstruktioner findes allerede en masse, så lad være med at forsøge at gøre koden til en referencemanual.

// TODO

Nogen vil måske mene, at vi her bevæger os ind i grænselandet mellem det ubrugelige og det brugbare, for hensigten med TODO-kommentarerne er naturligvis at gøre opmærksom på, at der er noget, der ikke er fulgt helt til dørs. Det er reelt nok, og jeg anvender selv TODO i min kode, mens jeg arbejder, men det er ikke i orden at checke kode ind med TODO-kommentarer.

Med mindre I bruger jeres repository til at planlægge arbejdsopgaver, er det jo et fjollet sted at registrere nye opgaver. Opret et work item, en incident, en change request eller hvad I nu kalder den slags og sørg for at få beskrevet, hvad der mangler, så I har en chance for at få fulgt op på det. Kildetekst er ikke særlig brugbar som projektstyringsværktøj.

Mange TODO-kommentarer er tilmed frygtelig indforståede. Jeg faldt over nedenstående under et code review for et stykke tid siden:


// TODO major hack!!!

Hvad forventes læseren at tænke her? Er nedenstående et major hack? I så fald hvordan og hvorfor? Mangler der er major hack her? Til hvad? Er der noget, der ikke virker? Det er bare ikke acceptabelt at gøre den slags.

Kommentarer som overskrifter

En hyppig anvendt fremgangsmåde er at bruge kommentarer som overskrifter for dele af koden. Udviklere, der anvender denne form for kommentarer, har ofte metoder, der er lange, men hvor kommentarerne formodes at give det gyldne overblik.

Således finder vi som regel kommentarer i stil med ”her initialiseres nødvendige strukturer”, ”her udregnes”, ”her udskrives”, ”her gemmes data” og så videre. Bagtanken er naturligvis, at læseren skal kunne tilegne sig et overblik ved at kigge ned over koden. Ideen er sympatisk men ikke særlig veludført.

Hvis en stump kode gør noget, vi kan sætte en fornuftig overskrift på, er det så ikke nærliggende, at denne funktionalitet eventuelt kan genbruges? Det gør kommentarer ikke det mindste for at fremme.

Et andet, større problem er, at ideen antager, at vi altid ser på koden i sin nuværende form, men hvad med de situationer, hvor vi virkelig har brug for at finde ud af, hvad der sker som f.eks. under fejlsøgning? Får vi en exception, er de velmenende kommentarer jo desværre ikke en del af vores stack trace. Her vil navnet på en kæmpe metode og et offset være ulig meget mindre værd end navnet på en lille, specifik undermetode.

I begge tilfælde er det meget mere anvendelig at skifte overskrifterne ud med metodekald. Måske endda metodekald på nye typer for derved at undgå, at hver type laver mere end en ting. Lav de enkelte blokke som metoder. Det giver korte metoder, der er lette at overskue og en god hierarkisk opdeling af opgaverne. Visual Studio gør det tilmed let at opdele metoder på denne måde via Extract Method. Ja, det giver flere metoder og flere kald, men kode, der er let at læse og vedligeholde, er altså uendelig meget mere værd end kompliceret kode, der måske kører en mikroskopisk brøkdel hurtigere.

De få tilfælde

Hvornår er det så okay at bruge kommentarer? Det korte svar er: ikke særlig ofte. Hver gang vi skal til at skrive en kommentar, bør vi i hvert fald lige tænke over, om der ikke er en bedre måde at udtrykke os i koden. Hvis kommentaren forsøger at lappe et hul, så lap hullet i stedet for at beskrive det. Hvis kommentaren forsøger at forklare kompleks kode, så skriv koden om, så den bliver forståelig. Hvis kommentaren forklarer en rodet struktur, så ryd op.

I mine øjne er kommentarer i kildeteksten kun brugbare, når de fortæller læseren noget, der ikke umiddelbart kan læses ud af koden. Forskellen på god og dårlig kode kendes som bekendt på antallet af WTF?!-udbrud hos læseren. Hvis læseren blot sidder og nikker og tænker, ”ja ja ja”, ”det giver mening”, ”sådan ville jeg også have lavet det”, er vi på rette spor.

Er vi tvunget væk fra dette spor, kan kommentarer være en hjælp til læseren. Det kan f.eks. være i tilfælde, hvor vi er nødt til at vælge en ikke oplagt løsning af hensyn til performance eller på grund af fejl, vi ikke har mulighed for at udbedre. Forklar læseren hvorfor det uventede giver mening i netop denne situation.

Klassebiblioteker

Et andet område, hvor kommentarer kan være anvendelige, er ved beskrivelse af offentlige typer og metoder i et klassebibliotek.
Det er fint med kommentarer, der forklarer intentionerne med og brugen af en type eller et interface (altså hvad og ikke hvordan), men det giver ikke rigtig nogen merværdi at få at vide, at et parameter ved navn filename indikerer navnet på en fil. Hvis læseren ikke kan læse navnet, er der ingen grund til at tro, at han kan læse kommentaren. Brug i stedet kræfterne på at vælge gode navne til metoder og argumenter og dokumenter så ideer og facetter, der ikke let kan læses ud af koden.

Der er ingen grund til at dokumentere alt blot for at kunne sige, at vi har dokumenteret. Kommentarer koster også tid og ressourcer at vedligeholde, så der er ingen grund til at skrive flere af dem end nødvendig. Ligesom kode kan kommentarer indeholde fejl, og fejlagtige kommentarer er som regel værre end ingen kommentarer. Hvis vi skriver få, meningsfyldte kommentarer, er det lettere at vedligeholde koden, end hvis vi på mekanisk vis forsøger at kommentere alt i detaljer.

DRY handler om mere end blot copy/paste

Friday, November 6th, 2009

For nogle år siden havde vi en lille, intern arbejdsgruppe, der havde til formål at komme med forslag til, hvordan vi kunne forbedre den måde, vi skriver kode på. Som oplæg læste vi blandt andet The Pragmatic Programmer, der er en fantastisk bog spækket med fornuftige tanker og anbefalinger. Har du ikke læst den, vil jeg opfordre dig til at sætte den højt på din to do-liste.

Et af de mange gode råd, der omtales i bogen, er DRY-princippet. DRY er en forkortelse for Don’t Repeat Yourself, og det går ud på, at vi ikke skal gentage os selv, når vi skriver kode. Ikke overraskende var det et af de råd, alle kunne tilslutte sig, for uanset programmeringssprog synes de fleste udviklere at være enige om, at copy/paste-kode er en rigtig dårlig ide. Strengt taget omhandler DRY dog mere end blot kodegentagelser. Ideen bag DRY er at undgå gentagelser i enhver form, men lad os holde os til kodegentagelser i denne omgang.

Desværre er problemet omkring copy/paste-kode ikke så interessant. Vi kan blive enige om, at det er en dårlig ide, og vi behøver sjældent at debattere hvorfor, det forholder sig sådan.

Den virkelige udfordring består i at identificere alle de steder, gentagelser optræder i mere subtile forklædninger. Lad os se på et forsimplet eksempel. Lad os forestille os, at vi har en klasse, Logger, som vi kan udskrive diverse driftsrelaterede beskeder til. Logger har altså en LogMessage(), der tager en string, så almindeligvis vil den blive kaldt som følger:

logger.LogMessage("Here is a message");

Det er der næppe noget odiøst i. Andre gange er beskeden dog mere omfattende, som i dette eksempel:

logger.LogMessage(string.Format("Current status {0}", GetStatus()));

I dette tilfælde har vi et yderligere metodekald knyttet til selve logging-operationen, og i princippet kunne der være et vilkårligt antal yderligere kald. Vi ved ikke, hvor lang tid det tager at udføre GetStatus() (samt eventuelle andre kald), men hvis det er en langvarig affære, er det ikke noget, vi er interesseret i at gøre, i fald logging er slået fra. Det spidsfindige er, at vi med den nuværende konstruktion kommer til at betale prisen for kaldet til GetStatus() uanset hvad, der end sker i LogMessage().

For at komme dette til livs, kan vi jo bare undersøge, om logging er slået til, inden vi laver noget. Så kunne det f.eks. se ud som følger.

if (logger.IsLoggingEnabled) {
   logger.LogMessage(string.Format("Current status {0}", GetStatus()));
}

Så undgår vi at kalde GetStatus() i de tilfælde, hvor det er overflødigt. Vi betaler godt nok en lille merpris som følge af vores tjek, så logging er blevet en anelse dyrere, men vi slipper for det overflødige arbejde.

Desværre har vi også netop introduceret en afhængighed, der gør, at vi kommer til at gentage os selv igen og igen. Vi har med andre ord brudt DRY-princippet!

Hver gang vi vil begrænse afviklingen af kode i henhold til det aktuelle niveau af logging, er vi nødt til at have denne IsLoggingEnabled / LogMessage()-konstruktion. Det er selvfølgelig kun to linjer, men det kan let blive til rigtig mange steder, og hvad sker, hvis kravene, til hvornår disse kald skal udføres, ændrer sig? Hvis vi er heldige, kan det lægges ind under vores IsLoggingEnabled abstraktion, men der er grænser for, hvor meget vi kan feje ind under det gulvtæppe. I værste fald må vi rundt og rette mange, mange steder, og det vil vi jo gerne undgå.

Problemet og en mulig løsning

Hvad er problemet? Vores API udstiller en afhængighed mellem to kald og tvinger således brugeren til at kæde disse sammen. Denne sammenkædning skal ske hver gang, vi ønsker at begrænse udskriften af log-beskeder og dermed får vi mange gentagelser. En sådan afhængighed bør håndteres af APIet og ikke af brugeren. Gør vi det, kan vi slippe af med gentagelserne.

Der er flere måder, at komme uden om dette på. Jeg vil dog nøjes med at se på en mulig løsning, for pointen er ikke at løse problemet i mit eksempel, men blot at illustrere problematikken.

For at begrænse de potentielt dyre kald, er vi nødt til at kunne slå afviklingen af disse fra, så vi har brug for en if-konstruktion, men da vi ikke ved, hvad det er, vi forsøger, at begrænse afviklingen af, er vi nødt til at indkapsle dette. Her kommer delegates os til undsætning. En delegate er blot en reference til et stykke kode, og denne reference kan vi naturligvis give som input til en metode. Med introduktionen af lambdaudtryk er det tilmed blevet meget nemt at gøre dette.

Vi kan altså implementere en LogMessage()-variant, som tager en delegate, der indkapsler alle de dyre kald, som input. Herefter kan vi lave det nødvendige tjek. Hvis og kun hvis logging er slået til, kan vi afvikle den kode, vores delegate peger på og logge den generede besked. Metoden kunne se ud som følger:

public void LogMessage(Func<string> func) {
   if (IsLoggingEnabled) {
      LogMessage(func());
   }
}

Vi kan f.eks. kalde LogMessage() med en delegate, der indkapsler det tunge arbejde således:

logger.LogMessage(() => string.Format("Current status {0}", GetStatus()))

På den måde skal den kaldende kode ikke længere tage stilling til, om logging er slået til eller ej, hvilket ikke alene gør koden simplere. Vi slipper også for gentagelserne. Hvis vi senere vil tilføje flere betingelser, har vi også et og kun et sted at gøre det.

Opgaven her var ikke at illustrere, hvordan et logging-API kan eller skal skrues sammen. Dertil er eksemplet for overfladisk, men forhåbentlig kan det illustrere, hvordan vi let kommer til at introducere gentagelser i koden, samt hvad vi kan gøre ved det.

Der er mange varianter af problemer i forhold til DRY, og dette indlæg har kun berørt en lille del. Hver gang vi bryder princippet, risikerer vi at gøre vores kode dyrere at vedligeholde, så derfor er det en god ide at komme gentagelserne til livs, inden de vokser sig store.

Tips og tricks til bedre kode

Wednesday, August 26th, 2009

Jeg er i skrivende stund i gang med en større gennemgang af en masse kode, og derfor er mine kodesensorer efterhånden helt oppe i omdrejninger. Når man sådan læser kode i dagevis, ser man både en masse godt og en masse skidt. I dette indlæg vil jeg se på nogle af de rigtig gode ideer, jeg er faldet over.

Eksplicitte Booleans

Vi er sikkert alle klar over, at sammenligning altid involverer to størrelser. En på hver side af sammenligningsoperatoren så at sige. Derfor kan jeg kun undre mig over, at folk skriver kode som f.eks.

if (OkToProceed) {
   // do something
}

Her er det underforstået, at variablen OkToProceed skal være sand, før vi fortsætter med do something. Compileren kan jo ikke læse vores variabelnavn, så hvordan kan vi være helt sikre på, at vi kun vil få udført vores kode når OkToProceed er sand? Er det ikke bedst at være sikre, helt sikre? Hvad hvis compileren tager fejl?

Jeg kan bedst lide, at intet er overladt til tilfældighederne, når det kommer til kode, og derfor gælder det om, at udtrykke præcis det, vi mener, i enhver sammenhæng. Ergo, hvis vi vil undersøge om OkToProceed er sand, skal vi skrive:

if (OkToProceed == true) {
   // do something
}

Derved er den ingen, der er i tvivl om, hvad vi mener, og vi garderer os effektivt mod de tilfælde, hvor compileren tager fejl.

I forbindelse med tildeling af bools, er det bestemt også en god ide, at være eksplicit, og her kommer ?: konstruktionen til sin ret. En bool kan let tildeles den korrekte værdi på følgende måde:

bool OkToProceed = IsEverythingOk ? true : false;

Så er det helt klart, hvad der foregår. Hver gang.

Lange metoder

Når vi fra en metode kalder en anden metode, er afviklingsmiljøet nødt til at gemme information om dette. Alle argumenterne til den kaldte metode samt en reference til den kaldende metode skal opbevares. Det sker på stakken, og det sker for hvert eneste kald! Som standard er stakstørrelsen i .NET 1 MB, hvilket jo ikke er meget. Tænk på, at den mindste iPod Nano har langt mere hukommelse, og den skal blot kunne spille lidt musik. Det siger således sig selv, at stakken er en meget begrænset ressource, og derfor er vi nødt til at udvise stor omtanke i brugen af denne. Bruger vi hele stakken, får vi en StackOverflowException, og som jeg har beskrevet andetsteds, hiver det hele processen ned. Det hele bliver kun værre af, at garbage collection ikke udføres for værdier allokeret på stakken. Her er vi selv nødt til at være på vagt.

Overfører vi mange argumenter, tager det plads, men det er der ikke rigtig nogen vej udenom. Vi skal jo bruge argumenterne. Derfor er vores eneste mulighed for at begrænse stakforbruget, at reducere antallet af metodekald. Den letteste måde at gøre dette, er ved at samle så meget logik i hver metode som mulig. Det betyder selvfølgelig, at vi skal kunne håndtere mange forskellige situationer i en og samme metode, men det er jo ikke for sjov, at vi har if, switch og så videre. Via den rette mængde flag, er det let at få en metode til at håndtere et væld af situationer, og derved kan vi undgå at fråse med den begrænsede stakplads.

En anden fordel ved de meget lange metoder er, at vi ikke skal huske så mange forskellige metodenavne, men det er selvfølgelig bare en lille bonus. Den egentlige gevinst består i, at vi med lidt rettidig omhug minimerer risikoen for StackOverflowExceptions.

Klargøring af variable

Apropos lange metoder, så er det sidste tip også særdeles relevant i den sammenhæng. Skønt store, omfattende metoder har sine fordele, som vi lige har set på, er der desværre en mindre ulempe ved dem. De mange linjer kode skal jo JIT-oversættes, og selvom compileren er ret effektiv, kan det tage sin tid. Derfor er det en god ide, at erklære alle variable i toppen af metoden, for det sikrer, at de bliver oversat først. Dermed kan vi være helt sikre på, at de er klar, når vi får brug for dem. Der er jo ikke noget værre end at få en NullReferenceException, bare fordi en given variabel ikke er blevet oversat, inden den skal bruges.

Det er ligeledes en god ide, at sørge for at genbruge de samme variable i videst mulig omfang, så vi derved får reduceret stakforbruget yderligere.

Konklusion

Jeg håber, du kan bruge ovenstående tips til at forbedre din kode, og jeg kan kun opfordre dig til at læse andres kode, da det er en glimrende kilde til inspiration. Har du selv gode tips, må du meget gerne skrive kommentarer til dette indlæg. God fornøjelse!

Find masser af fejl på en dag

Friday, July 24th, 2009

Inspireret af Poul-Henning Kamps projekt med at udgive kildeteksten til Varnish i bogform, besluttede for nylig at hælde alle vores kildetekster sammen i en stor fil og hive denne ind i en brugbar editor.

Herfra kunne jeg ret hurtig søge efter specifikke konstruktioner og foretage diverse stikprøver. Formålet med øvelsen er ikke at forsøge at få overblik over vores knap en million linjer kode. Det er en ret fjollet Sisyfos-opgave, som jeg vil overlade til andre, men ved at se på koden på den måde, kommer man ofte til at bemærke facetter, som måske ikke var evidente, da koden blev skrevet. Ved at være ligeglad med overblikket, kan man koncentrere sig om detaljerne.

Jeg benyttede følgende lille liste, og fandt mange eksempler på fejl og uhensigtsmæssigheder i vores kodebase:

  • Søg efter == 0.0. Sammenligning af doubles/floats med 0.0 er typisk en dårlig ide, da koden sjældent vil opføre sig som forventet.
  • Søg efter throw efterfulgt af andet end ; og n (som i new). Det giver steder, hvor exceptions muligvis bliver ”rethrown” på den uheldige måde.
  • Søg efter linjer, der begynder med ~, altså finalizers. Den efterfølgende gennemgang er lidt mere tricky, da vi typisk vil skulle bruge lidt mere end et par sekunder på at se, om den implementerede finalizer er fornuftig. Mit forslag er brug et minut eller to på hver. Der skal helst ikke være for mange af dem, så det burde være en overskuelig opgave. Målet er at identificere de finalizers, der er åbenlyst forkerte. En oplagt ting at undersøge er om finalizers kan smide exceptions (det skal de ikke!).
  • Søg efter void Dispose(. Sammen historie som ovenfor. Husk at se om Dispose() kalder GC.SuppressFinalize(this), hvis typen også implementerer en finalizer. Husk også at Dispose() ifølge konventionen skal kunne håndtere at blive kaldt flere gange på samme instans.
  • Søg efter new Thread(. Mange opgaver vil med fordel kunne løses af thread poolen frem for via egne tråde. Det er ret dyrt at lave nye tråde, så derfor bør man flytte alle de opgaver, der kan varetages af thread poolen væk fra egne tråde.
  • Søg efter .Close( og .Disconnect( – typisk vil disse kald være overflødige, hvis den aktuelle type implementerer IDisposable, og bliver anvendt i en using-blok. Der kan være grunde til ikke at bruge using, men i så fald skal kald af Close/Disconnect placeres i en finally-blok.
  • Søg efter .Abort() – det er sjældent en god ide, at kalde Abort() på en anden tråd.
  • Søg efter lock(this og lock(typeof – begge er offentlige referencer, og derfor er der risiko for deadlocks.

Ovenstående er ikke ingenlunde en komplet liste af potentielle problemer, men det er eksempler på mulige fejl, der kan identificeres med en meget begrænset indsats. Yderligere forslag modtages gerne.

Skønt fremgangsmåden producerer falske positiver, er de relativt lette at identificere, og derefter har man en liste af konstruktioner, der er værd at se nærmere på. Det hele skal helst ikke tage mere end nogle få timer.

På en større kodebase vil jeg ikke bliver overrasket over at finde nogle hundrede eksempler på kode, der kan føre til alvorlige fejl. Hvis bare halvdelen af disse er reelle problemer, er afkastet af investeringen særdeles gunstigt.

Efter at have søgt koden igennem, samlede jeg reglerne i et lille Perl-script, så jeg let kunne generere en rapport over observationerne.

Disclaimer

Ovenstående er hverken genialt eller revolutionerende på nogen måde. Det er i bedste fald den fattige mands udgave af, hvad værktøjer som R#, CodeRush, FxCop og så videre kan gøre meget bedre. Har man allerede disse værktøjer implementeret, er der således ikke meget at komme efter her. Når jeg alligevel synes, at det er et indlæg værd, er det fordi, at den slags værktøjer ikke altid er i brug over hele linjen.

R# og CodeRush koster penge og flere brugere beklager sig over den reducerede hastighed i Visual Studio ved brug af disse. På min arbejdsplads er det f.eks. langt fra alle, der benytter R# til trods for, at vi har muligheden.

FxCop er godt nok inkluderet med Visual Studio, men det har en forholdsvis omstændelig indkøringsperiode. Der er meget at sætte sig ind i, og signal/støjforholdet kan være problematisk, når man kobler værktøjet på en eksisterende kodebase. Jeg vil påstå, at det vil tage dage, hvis ikke ligefrem uger at få noget ud af FxCop. Hvis man har den tid, er det en god investering, men det er ikke alle der har den mulighed.

Ovenstående teknik kommer altså ikke i nærheden af at kunne konkurrere med dedikerede værktøjer, men den er let og billig at gå til, og hvis man ikke har andre processer/værktøjer i spil, vil den højst sandsynlig finde et antal fejl med en meget begrænset indsats. I værste fald må man nøjes med at være blevet lidt klogere på kollegernes kodestil, hvilket faktisk også kan være ganske lærerigt.

var eller ej, det gør næppe den store forskel

Thursday, May 14th, 2009

C# 3 introducerede som bekendt et væld af nye features og muligheder. Der er allerede skrevet rigtig meget om LINQ, extension methods og så videre, men hvis der er en enkelt feature, der i den grad er blevet overeksponeret, er det nye var-keyword. Med jævne mellemrum synes en eller anden blogger, at det er tid til en opsang omkring brugen af var, og senest har Microsofts danske udviklerevangelist meldt sig i koret af folk, der vil advare mod de grelle konsekvenser ved brugen af dette udskældte keyword.

Jeg ser ikke noget formål i at forsøge at argumentere for, hvorfor jeg ser var som en genial, lille feature. I stedet vil jeg blot slå fast, at jeg bruger var til stort set alle lokale variable, og jeg glæder mig derfor over, at ReSharper understøtter mine præferencer på det punkt.

Den interessante debat er ikke, om var gør koden mere eller mindre læselig. Det interessante er, at brugen af var eller ej ikke har nogen nævneværdig indflydelse på læsbarheden af koden. Bevares, vi kan argumentere for, at der er en forskel, men uanset hvilket synspunkt vi tilslutter os, er der næppe nogen af os, der vil påstå, at brugen af var er den altafgørende forskel mellem læsbar og ulæselig kode.

Tager vi let læselig kode og skifter alle eksplicitte erklæringer ud med var, vil nogen hævde, at koden bliver en smule pænere, mens andre vil argumentere for, at den bliver lidt mindre læselig, men der er næppe nogen, der vil påstå, at den før læsbare kode nu er helt ulæselig. Analogt kan vi tage rodet, ulæselig kode og enten indføre eller eliminere brugen af var, uden at det vil udrette noget væsentlig. Ja, der er en forskel, men nej den er ikke så udslagsgivende, at var fortjener al denne opmærksomhed. 

Jeg har allerede skrevet lidt om, hvad jeg betragter som vigtige egenskaber ved læsbar kode, men for blot at repetere et par pointer er sigende navne til variable og metoder samt korte, velafgrænsede typer og metoder langt vigtigere for læsbarheden end brugen af enten implicitte eller eksplicitte typeerklæringer. var er og bliver altså ikke det store problem. 

Så til alle jer, der råber vagt i gevær, hvis I skal komme med gode råd til hvordan vi kan skrive bedre kode, er var altså ikke det rette sted at begynde.