null-referanser
- Sunday, June 21st, 2009
- Skriv en kommentar
På flytoget etter NDC 2009 hadde mine kollegaer og jeg en nokså heftig diskusjon om bruk av null. Det var et nokså uskyldig spørsmål som startet det hele:
“Sjekker du alltid om parametrene som sendes inn til en metode er null, og kaster ArgumentNullException om så er tilfelle?”
Mitt svar var ganske bestemt: Nei, det gjør jeg stort sett aldri! For meg er det å sjekke for null noe man gjør i frykt, noe man gjør for å kompansere for dårlig kode. I de fleste tilfeller er det en bug å sende inn null, og jeg ønsker ikke å legge til rette for sloppy kode ved å sette på støttehjulene og teste for dette.
Kode som forventer en instans men får null vil kaste en NullReferenceException, som er mer enn godt nok for kode som er godt skrevet - den energien en eventuelt putter inn i å lage mer detaljerte feilmeldinger er ikke verdt det i dette tilfellet.
Videre hevdet jeg at null i seg selv er en uting. Kode som er skrevet slik at man gjentatte ganger må teste for null er ikke god kode. Å teste for null er en code smell. Bruk av null hører ikke hjemme i kode som befinner seg andre steder enn på det laveste abstraksjonsnivået - null er ikke noe forretningsfolk kan snakke om, og bør derfor etter min mening heller ikke eksistere i kode som beskriver forretningskonsepter.
Null Object pattern
Problemet jeg snakker om kan illustreres med følgende kodesnutt:
Employee e = DB.GetEmployee(“T-Man”);
if (e != null && e.IsTimeToPay(DateTime.Today))
e.Pay();
Siden databasen kan returnere null om “T-Man” ikke eksisterer benytter vi oss av det faktum at det første uttrykket i if-testen evalueres først, og det andre uttrykket kun om det første er sant. Vi har alle skrevet kode som dette her. Vi har også alle glemt å teste for null fra tid til annen - selv om dette er et vanlig mønster så er det både uelegant og noe som fører til mange feil.
I stedet for å returnere en null-referanse kunne vi la databasen kaste en exception, men da måtte vi ha brukt try/catch ved alle kall, noe som er enda mindre elegang.
Et alternativ til å bruke null-referanser er å implementere et objekt som har det forventede grensesnittet, men som ikke gjør noen ting. Dette kaller vi for Null Object. Min kollega hevdet at dette er veldig tungvindt å implementere, men etter å ha tenkt meg litt om så finner jeg mange steder hvor jeg har brukt det selv, og hvor det er langt fra vanskelig å implementere, samtidig som det gjør koden som bruker objektene langt enklere.
Her er et eksempel fra prosjektet jeg holder på med akkurat nå. Brukeren skriver inn noe tekst som blir gjort om til en UserCommand. Poenget er å unngå å returnere null, så hvis teksten som sendes inn er gibberish så returnerer jeg en UnknownCommand:
public UserCommand GetCommand(FirstPerson person, string[] commandArguments)
{
if (ValidCommandArguments(commandArguments))
{
return _commandDictionary
.GetParser(commandArguments[0])
.GetCommand(person, commandArguments);
}
return new UnknownCommand();
}
UnknownCommand er et null-objekt: Det implementerer UserCommand-interfacet, men gjør ingen ting. Klienter av UserCommand kan bruke dette objektet på samme måte som alle andre commands - jeg trenger ikke kaste noen exceptions, og aldri teste for null.
public class UnknownCommand : UserCommand
{
public UserCommandResult Execute()
{
return new UserCommandResult
{
Success = false,
Text = “Did not compute!”,
};
}
}
Jeg har flere eksempler som ligner på dette. Der hvor jeg bruker strategy pattern i kombinasjon med en eller annen form for en factory, implementerer jeg nesten alltid også et null-objekt, en dummy-klasse som kan brukes i stedet for en virkelig strategi.
Dette beviser ikke at null objects kan brukes over alt, ei heller at det er trivielt, men jeg er ganske sikker på at bruk av dette mønsteret vil føre til kode som er enklere å lese og vedlikeholde, og som er mer feilfri.
Kan vi erstatte != null testen?
Men det er ikke til å komme bort ifra at man av og til må bruke null-referanser. Det er også et faktum at jeg hater å skrive != og == null. Jeg ønsker derfor en mer deskriptiv måte å teste på - en som ikke bryter abstraksjonsnivået. Dette er hva jeg har kommet frem til:
[Test]
public void Test()
{
object o = null;
if (o.IsNotSetYet())
Assert.That(o == null);
//Cool, I called a method on a null reference
else
Assert.Fail(“o was not null”);
}
Denne testen feiler ikke! Men hvordan er det mulig å kalle en metode på en null-referanse? Jo, det er mulig takket være extension methods:
public static class ObjectExtensions
{
public static bool IsNotSetYet(this object o)
{
return o == null;
}
public static bool IsSet(this object o)
{
return o != null;
}
}
Er det ikke vakkert?
Kategorier: OO-design/clean code.
RSS feed for kommentarene.
Tilbaketråkk.
June 21st, 2009 at 11:04 am
Veldig interessant! Er absolutt enig i at null i koden er en uting.
Men:
Jeg vil argumentere for å ikke erstatte != null der hvor man faktisk trenger det, fordi dette er mer leselig. Ja, du leste riktig. MER leselig rett og slett fordi vi programmerere er så vante med å bruke det. En utvikler som ser koden din for første gang vil ikke nødvendigvis vite hva som skjer der, og da virker det mot sin hensikt.
Et spørsmål til koden din:
Å ha både IsSet() og IsNotSetYet() er vel kodeduplisering?
Så en kommentar til extension methods:
Utvidelsesmetoder er virkelig kraftige saker. På NDC nå kommenterte jo selveste Robert Martin at dette var nyttig i noen sammenhenger, men at det er lett å gå feil. For eksempel er det ekstremt viktig at man plasserer slike utvidelsesmetoder i riktige assemblies. Problemet blir jo at når man refererer et assembly som inneholder utvidelsesmetoder at det er lett å bli bundet opp til assembliet pga utvidelsesmetodene i assembliet og ikke pga det man egentlig trengte assembliet for. Så hvor bør man plassere ObjectExtensions? Sannsynligvis i en assembly som all koden din bruker uansett. Eller?
June 21st, 2009 at 2:19 pm
Takk for kommentarene, Halvard. Her er mine..
“En utvikler som ser koden din for første gang vil ikke nødvendigvis vite hva som skjer der”
Det gjør overhodet ingen ting. Som utviklere bør vi søke høyere abstraksjoner, og fjerne oss fra den implisite koden vi bedriver. Skriv kode som forteller “hva” og “hvorfor”, ikke “hvordan”. Når vi ser kode som i += 1; så tenker vi ikke på hvilke maskininstruksjoner som må til for å få til dette - vi har gått et steg videre. Og jo høyere abstraksjonsnivå vi kan tenke i, jo mer vil vi får gjort. Hvis jeg skriver kode som customer.IsReallyACustomer() så behøver ikke du å bry deg om at det er implementert som customer != null på nivået under. Ikke før du beveger deg ned på det nivået.., og da ser du det.
!= null er noe vi er vandt til, og derfor er det leselig, og vi ønsker å beholde det vandte og trygge. Men skal vi utvikle oss så må noe vike.
“Å ha både IsSet() og IsNotSetYet() er vel kodeduplisering?”
Ikke mer duplisering enn å ha både == og !=. I stedet for a!=b burde vi skrive !(a==b). Og hvorfor har vi både && og ||? Vi kan klare oss lenge med bare || og litt negering, kan vi ikke?
Ved å ha IsNotSetYet() så slipper jeg å skrive !o.IsSet(), som er helt uforståelig for alle andre enn dem som er vandt til et programmeringsspråk som benytter ! for negering. For ikke å snakke om hvor lett det er å overse det ene tegnet. Nei, den type duplisering, om du vil kalle det det, er jeg ikke redd for.
“Så hvor bør man plassere ObjectExtensions?”
Det er et viktig spørsmål. For tiden har jeg noen små “Marosoft.*” assemblies jeg referere overalt, som inneholder helt generiske ting - og slik jeg bruker dem passer ObjectExtensions inn der. Jeg ser på dem som en extension til System-namespacet.
Men jeg er usikker på om jeg vil bruke disse generiske IsSet og IsNotSetYet, eller om jeg vil lage mere domene-spesifike versjoner, og da plassere dem der hvor de hører hjemme.
Det Uncle Bob har å si om komponenter er utrolig spennende (ikke at jeg ikke sluker rått alt det andre han sier også).
June 21st, 2009 at 2:54 pm
Misforstå meg rett, jeg sier ikke at vi kun kan referere null i mine to extension methods, og bruke dem alle steder hvor vi bruker null i dag. Det ville ikke være noe spesielt fremskritt. Og jeg har ingen erfaring med disse metodene enda heller.., så la meg eksperimentere litt først, så ser vi hvor jeg ender
.
Vi er enige om at mye null i kode er en uting, som var budskapet mitt..
June 21st, 2009 at 3:07 pm
Gode svar, jeg er veldig tilbøyelig til å være enig!
Angående kodeduplisering;
1.
Ville du også lagd en metode customer.IsReallyNotACustomer() ?
2.
Du har jo duplisert koden inni metodene IsSet og IsNotSetYet (selv om de tilsynelatende gjør motsatte ting)?
Vet du hvordan det virker i extension methods, kan man f.eks. skrive slik
public static bool IsNotSetYet(this object o)
{
return ! IsSet(o);
}
for å unngå kodeduplisering?
June 21st, 2009 at 5:27 pm
Punkt 2 først: Mener fortsatt det er feil å si at jeg har duplisert kode, men jeg skjønner hva du mener. Forslaget ditt fungere om du endrer det til return !o.IsSet();
Punkt 1, om jeg ville ha laget en IsReallyNotACustomer? Det kommer an på om jeg trengte det for å skrive lesbar kode. Kanskje har jeg behov for å sjekke at noe ikke er null, kanskje er behove å sjekke at det er null, kanskje er det begge deler. Jeg skriver ingen av dem før jeg ser at jeg behøver dem.
June 23rd, 2009 at 1:11 pm
Glenn Block advarte også om over-ivrig bruk av extension methods på generelle objekter i foredraget om framework design guidelines. Han sa bl.a. at det ikke bør benyttes på object. Mulig IsNotSetYet()/IsSet() kan være et unntak siden dette er en helt generell sjekk, men er det så mye penere enn å sjekke direkte for null da?
June 23rd, 2009 at 1:32 pm
Nja, jeg vet ikke om det er så mye bedre. Men nå har jeg eksperimentert litt mer, og ser at jeg oppnår veldig lesbar kode. Se for eksempel her:
public UserCommandResult Execute()
{
var item = GetItem();
if (item.WasNotFound()) return ItemNotFoundResult();
if (item.CanNotBeCarried()) return ItemCanNotBeCarriedResult();
PickUp(item);
return SuccessResult();
}
Her skjuler det seg to null-tester.., men de er implementasjonsdetaljer. Koden lar seg nå lese som en god historie. \”If item can not be carried return item can not be carried result.\” Føles veldig riktig for meg.
Update: Merk at dette ikke er generelle extension methods for object. WasNotFound() skjekker derimot enkelt og greit om Item == null, mens CanNotBeCarried() bruker \”as\” nøkkelordet i C# til å kaste item til ICanBeCarried interfacet, og sjekker om resultatet av det er null. CanNotBeCarried() vil jeg ikke plassere på Item direkte, item er lukket for den type endringer. Etterhvert som jeg legger til flere item-egenskaper i form av ulike interfacer kan jeg legge til flere slike metoder uten å måtte endre Item.