fredag 11. desember 2009 WTF
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..
17 /* W A R N I N G :
18 *
19 * Crappy design - need to fix ASAP !!!
20 * padloc static initalizer now needs to come before _formatterFactory initializer,
21 * since it is needed in static CatewayConfig which is used there.
22 */
23 private static readonly object padlock = new object();
24
25 private static MessageFormatterFactory _formatterFactory
26 = new MessageFormatterFactory(GatewayConfig[GatewayConfigKeys.MessageFormatterStrategy]);
27
28 private static Dictionary<string, string> _gatewayConfig;
29 public static Dictionary<string, string> GatewayConfig
30 {
31 get
32 {
33 if (_gatewayConfig == null)
34 lock (padlock)
35 if (_gatewayConfig == null)
36 _gatewayConfig = RepositoryFactory
37 .Get<GatewayConfigRepository>()
38 .GetGatewayConfig();
39
40 return _gatewayConfig;
41 }
42 }
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.
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 :)