Først en advarsel: Det som følger er ikke representativt for koden vi produserer her på kontoret. Og jeg er selv ansvarlig for akkurat dette makkverket – ikke mitt stolteste øyeblikk – så jeg henger ikke ut noen andre enn meg selv. Like fullt, denne snippeten kan illustrere et poeng..

Dette er en bit fra en statisk klasse som fungerer som en sentral ressurs for å opprette meldingskøer. Klassen holder en statisk referanse til en Dictionary med konfigurasjonselementer kalt _gatewayConfig – denne aksesseres via GatewayConfig propertien, som er ansvarlig for å opprette den første gang. Den inneholder også en MessageFormatterFactory som initialiseres ved hjelp av GatewayConfig. Og her finner vi det skumle..

Initializers – dvs. inline initialisering av variabler før konstruktører kjører – kjøres sekvensielt i den rekkefølgen de er skrevet i koden. Hvis initaliseringen av padlock i linje 23 flyttes til linje 27 – til etter initialiseringen av _formatterFactory – vil dette ikke fungere i det hele tatt, fordi padlock da vil være null når den trengs i GatewayConfig propertien. Ikke helt enkelt å få med seg det første gang man leser koden for å si det sånn. Ta gjerne en titt til.

Koden fungerer altså slik den er presentert her, men når vi har gjort oss avhengig av rekkefølgen variablene i en klasse deklareres er vi ille ute, dette er et sårbart design. Heldigvis fanget jeg opp problemet denne gangen med vår eksellente samling med automatiserte tester.

Statiske konstruktører

Det er ganske vanlig praksis å initiere statiske variabler inline som i koden over. .NET gir deg derimot også muligheten til å definere klasse-konstruktører (statiske konstruktører) for dette formålet. Og i dette tilfellet – gitt at vi ikke skal gjøre en større refakturering – er den naturlige løsningen å bruke en slik konstruktør.

Om jeg gjør det gir derimot kodeanalysen i Visual Studio meg følgende advarsel:

CA1810 : Microsoft.Performance :
Initialize all static fields in ‘MessageQueueFactory’ when those fields are declared and remove the explicit static constructor.

Så kan altså være en performance penalty knyttet til å bruke statiske konstrultører?! Mer om dette rådet fra Microsoft her: Initialize reference type static fields inline. Jeg har lest teksten flere ganger, og er ikke sikker på at jeg skjønner problemet helt. Vel, nå har jeg i alle fall deligert oppgaven med å løse problemet til en annen på teamet, så får vi se hva han kommer opp med :)

Noe av det første man lærer seg som utvikler – eller i alle fall noe av det første man bør lære seg – er at man ikke skal repetere kode. Informasjon skal finnes ett og kun ett sted. Vi kaller prinsippet for DRY, og slik jeg ser det er dette hele grunnprinsippet med programmering. I stedet for å gjenta manuelle prosedyrere gang på gang, automatiserer man det gjennom å sentralisere kunnskapen i programkode.

Fordelene man oppnår gjennom å følge DRY riktig, er at koden enklere lar seg både lese og endre, og man unngår feil som oppstår om man gjør endringer ett sted men glemmer et annet.

På jobben har vi mye repeterende kode, og desverre er det bare ett av problemene våre. Jeg kommer stadig over stygge ting i produktet vårt, og i dag fant jeg følgende kodesnutt. Read it and weep!

Det er så mange ting galt her at det er vanskelig å vite hvor man skal begynne. Det mest åpenbare er at vi bruker en funksjon som tar 39 parametre. Og svært få av dem er i bruk, slik at vi repeterer null 143 ganger. Vi benytter kodegenerering basert på database-skjemaet vårt, og metoden med 39 parametre er en av disse som lar deg gjøre spørringer mot et view.

Dette er overhodet ingen unnskyldning.., det er knapt nok en forklaring. Kodegenerering skal sørge for at vi kan unngå repeterende kode, ikke føre til mer kode!

Problemet jeg følte et øyeblikkelig behov for å fikse var derimot repeteringen av kallet til denne stygge metoden. Hvorfor kunne man ikke ha tatt seg bryet med å fikse dette? Ikke at løsningen min (se under) er spesielt mye finere å se på, men det er i alle fall litt enklere å forholde seg til.

Nå kaller vi i alle fall den styggeste metoden bare én gang, og vi har sentralisert den stygge logikken for documentTypeID og mikValid til ett sted (don’t get me started on why it has to be like that). Og med gjeldende formatering har vi til og med gått fra 46 til 37 linjer, selv om vi har laget en ny funksjon. Jeg har mye å gjøre her, men dette var i alle fall en begynnelse…

Jeg er klar over at dette ikke er det perfekte eksempelet å bruke for å forklare DRY-prinsippet, det var bare tilfeldigvis den kodesnutten som irriterte meg mest i dag. En grei start på min nye blog-kategori som jeg har valgt å kalle WTF!

PS: Jeg har ikke publisert denne koden for å henge ut utviklerne jeg jobber med, men for at de som leser dette her skal lære noe. Og for at man skal få seg en god latter. Jeg håper disse to tingene lar seg kombinere uten at noen føler seg støtt.

For tiden er jeg opptatt med å løse noen problemer vi har i produktet vårt relatert til dokumenthåndtering. Vi lagrer dokumentene som blobber i databasen, og de siste dagene har jeg kommet over flere eksempler på hvor lett det er å gjøre ting feil i slike senarier.

WTF 1:

For å håndtere store filer er vi nødt til å splitte dem opp – vi kan ikke behandle de komplette filene i minne. Før vi splitter opp ønsker vi å finne størrelsen på filen. Og hva gjør vi da? Jo, vi laster hele filen i minne og sjekker lengden!

Det verste er at filstørrelsen allerede finnes lagret i et eget felt i basen, men av en eller annen grunn stoler vi ikke helt på denne, og velger å gjøre det på den kostbare måten som får serveren til å knele for store filer.

WTF 2:

Et annet, morsomt eksempel vi kom over i går var koden for å slettemarkere en fil. Alt som skal gjøres er å flippe en bit fra true til false (eller var det muligens motsatt?). Men for å gjøre det laster vi først hele raden fra databasen, som desverre også inkluderer fil-blobben. Så hvis noen for eksempel vil slettemarkere en fil på 100MB, så leser vi først 100MB fra databasen før vi endrer vår bit fra 1 til 0 og oppdaterer.

Nå må ingen føle seg støtt av disse historiene. Alle kan gjøre slike ting fra tid til annen – de fleste, om ikke alle, gjør slike ting fra tid til annen. Men i et stort prosjekt i stadig utvikling er det viktig å sette av tid til å fange opp og jobbe med problemer som disse – spesielt når det dreier seg om produktets absolutte kjernefunksjonalitet.

Jeg garanterer en stor forbedring i neste patch!