Nja, vilka skolor tänker du på nu? Man får använda sunt förnuft. En sak som kan vara avgörande oxå är ju om du vill exponera din orderlinecollection utåt.... Jag tycker egentligen ingen av dem. Jag tycker nog AddProduct den berättar ju mest. Instämmer helt med Johan! AddProduct kan blir språkligt helt fel om företaget säljer tjänster men aldrig några produkter. >Läsbarhet i all ära men inte på bekostnad av smarthet :) Niclas: >Jag kör ofta på Alternativ 1, Keep It Simple Stupid (KISS), om jag ändå har en lista av OrderLines, >varför inte utnyttja den, varför skapa en metod på Order för att lägga till en order rad.. så tänker >jag.. men som sagt, beror helt på hur kundens modell ser ut, inte vad jag tycker. Man kan se KISS i detta context på följande sätt också: Största vinsten med LoD vad jag tycker är tydligheten, och inkapslingen där man inte låter någon få gå över en gräns för att nå något annat. Även felhantering kan blir både bättre och mer tydlig då man appliceras business med defensive programming i aggregatrootets metod. Mycket bryter här :). Felhanteringen går ju att lösa om OrderLineCollection känner till sin ägare. Ibland ser man inte träden för all skog... Jag ser fördelarna, det finns där för lösa problem som jag tycker är logiska...Men det betyder inte att jag har mer rätt än någon annan... En sak som jag kom tänka på när det gäller "hur tänker ni?": Angående diskussionen om att alternativ 1 bryter mot Law of Demeter men inte alternativ 2, så kan det också vara värt att notera att alternativ 2 inte är det enda sättet att undvika bryta mot LoD, och att man inte behöver lägga till en ny metod på Order-klassen för att klara LoD. @Tomas: Tomas:Hur tänker ni?
order.OrderLines.Add(new OrderLine(product));
order.OrderLines.Add(orderLine);
eller
Order.AddLine(product);
Vad jag har fattat det som så säger den gamla skolan alternativ 2, men den nya alternativ 1.
OrderLines i detta fallet skulle då vara en OrderLineCollection.
Vad tror/säger ni?Sv: Hur tänker ni?
I ditt exempel är det väl helt OK med:
order.OrderLines.Add(new OrderLine(product));
Däremot om det börjar växa till något i stil med:
order.OrderLines.Add(new OrderLine(new product(a,b, new Supplier(theSupplierId))));
Då är det väl inte så fräckt längre.. :)
Det blir svårare att läsa koden och svårare att debugga.
Man ska inte göra det svårt för sig i onödan.
Poängen med programmering är ju inte att det ska vara så få rader som möjligt :)
Det finns inte heller någon prestandafördel med sådana förkortningar.Sv: Hur tänker ni?
Sv: Hur tänker ni?
Detta är för mig mer rätt:
order.AddProduct(product,quantity);
Det är egentligen rätt lät att se om man har bra kod genom att läsa den och försöka berätta den som en saga.
ta dina exempel:
>order.OrderLines.Add(new OrderLine(product));
en produkt läggs till en ny orderrad som läggs till sina orderrader till sin order.
>order.OrderLines.Add(orderLine);
en Orderad läggs till sina orderrader till sin order.
>Order.AddLine(product);
en produkt läggs till i en ny rad (vad för rad?) till sin order.
>order.AddProduct(product,quantity);
en produkt med dess antal läggs till sin order
Vilken beskrivning anser du vara bäst och enklast att förstå? Samt mer story berättande?
Mvh JohanSv:Hur tänker ni?
Dock så beror det ju på hur utvecklaren helst vill arbeta men AddProduct är nog bäst.
Denna tråd var väldigt lik min förra som gick lite mer in på djupet så jag stänger denna tråden.Sv:Hur tänker ni?
Att skapa tydliga interface som är "självdokumenterande" gör verkligen underverk för kodens förvaltningsbarhet. ( och då kan man ju skippa att skriva all trist dokumentation som ändå ingen törs lita på efter det att koden är proddad :)
Udi Dahan har en rätt bra post ang. "fluent interfaces" som jag kan tipsa om!
http://udidahan.weblogs.us/2008/02/15/from-crud-to-domain-driven-fluency/Sv:Hur tänker ni?
AddDetailLine() tycker jag funkar bättre. Detaljer till en order kan vara tjänster, produkter, men även annan information som rabatt, leveransinformation osv. Om en programmerare inte kan begripa vad AddDetailLine kan tänkas innebära kanske de borde göra något annat än just programmera.
Läsbarhet i all ära men inte på bekostnad av smarthet :)Sv: Hur tänker ni?
öööö ok...
smarthet för vem? Ser inte ens argumentet som speciellt smart tänkt...
"Om en programmerare inte kan begripa vad AddDetailLine kan tänkas innebära kanske de borde göra något annat än just programmera. "
Du måste mena, kan inte programmeraren förklara för andra programmerare vad ens saker gör BÖR han/hon inte vara programmerare...
Det namn man bör välja är det namn kunden pratar... Pratar han/hon om detaljlijer så fine... Men pratar han/hon om tjänster och produkter bör man kunna hantera båda två som egna entiteter och inte slå ihop dem till något påhittat...
mvh JohanSv: Hur tänker ni?
Alternativ 1 skulle bryta mot Law of Demeter (http://en.wikipedia.org/wiki/Law_of_Demeter) men inte Altenativ 2.
Jag själv bryter ofta mot Law of Demeter, och i ditt fall så tycker jag inget av alternativen är fel, allt beror på domän modellen och den kan variera från kund till kund, beror på vilket språk dom pratar etc.
Man kan vrida och vända på detta på olika sätt, tex se alternativ 2, Order.AddLine(produkt).. kan en order verkligen lägga till en egen rad? Så ska vi låta Order göra det eller någon annan?
Ser vi på alternativ 1, så kan det tolkas av olka på olika sätt, men här är ett sätt:
Via Order kommer vi åt OrderLines, och vi lägger till en ny OrderLine till OrderLines. Här lägger inte Order till något, utan något läggs till på OrderLines.
Jag kör ibland med Alternativ 1, Keep It Simple Stupid (KISS), om jag ändå har en lista av OrderLines och inte behöver göra något spec när jag lägger till en OrderLines så utnyttjar jag den, varför skapa en metod på Order för att lägga till en order rad om det inte behövs, men det beror helt på hur kundens modell ser ut, och det är efter den via ska utveckla.
Ola:
"Om en programmerare inte kan begripa vad AddDetailLine kan tänkas innebära kanske de borde göra något annat än just programmera."
Då måste jag nog sluta programmera för AddDetailLine är för mig defust
Mvh Fredrik Normén
MVP - MEET - ASPIndiser
http://weblogs.asp.net/fredriknormenSv:Hur tänker ni?
Skönt att bröder inte känner lika om allt :-)
Men Alt1 anser inte jag vara direkt KISS, dvs det är stupid med inte möjligen benämningen Simple, dvs simple i denämningen KISS baseras mer på (om jag inte förstått det fel) att det skall vara enkelt för andra. Att körta chain kod, typ bryta LoD gör det inte alltid lättare för andra.
Order.OrderLine.Add(...) är i senariot kanske inte bästa exemplet, man får tänka ett steg längre här, tillåter man kod med chain så kommer man tillåta det över allt i sin kod, det är där problemen kan uppstå.
Ett exempel som tydligt förklarar Stupid.
//wag the dog tail
dog.body.tail.Move()
Det är simple att skriva men det är långt från stupid dvs vara så korkat lätt att förstå.
Att basera sin kod efter LoD skulle ge:
dog.WagTail(); istället... Det är både enkelt att förstå, det är även korkat i det tekniska ansenede då koden innan kunde skrivits och gjort samma sak. Men koden ovan har en del brister.
Dels är det inte så självförklarande vad man gör. Förutom kommentaren. Sen så HOPPAR man över intärn kapslade klasser dvs du går från dog till Tail. Om något skulle gå fel ex body är null, eller tail är null så får man ett väldigt dumt fel som bara pekar på denna rad och en del arbeta kan behöva läggas ner för debbuging i onödan för att ta reda på vilket objekt är egentligen null och varför.
Mocking blir svårt, ungefär samma argument som att static inte är så bra alltid då de inte går att mocka bort. Enligt regler för enhetstest skall man kunna testa enheten utan att gå utanför dess bounderies, dvs Hundens svans skall gå att vifta på. Skall man då testa Tail.Move eller Dog.WagTail?
testar du tail.move så testar du bara en svans men inte just den hundens.
m.m.
Nu är det sköna med LoD och övriga principer att man måste inte följa dem om man inte gillar dem... :-) Jag gillar LoD... Men smaken är som baken... Sen hjälper den andra som kanske inte är så bra på koddesign att ha lite bättre design...
Det finns de som faktiskt använder många fler än . steg i sin kod.
A.B.C.D.DoStuff[0].E.F.G(foo);
O tycker det är så självklart det bara kan bli bara för att det går att göra så....
Mvh JohanSv: Hur tänker ni?
Keep it simple, i detta fall är order.OrderLines.Add simpelt, behöver inte skapa mer kod, allt finns redan och är enklet att använda.. sedan om LoD är bättre så vore ju OrderLines.Add stupid, bara en tanke ;)
Det finns för och nackdelar med LoD. LoDs stora nackdel är alla wrapper metoder som kommer att behövas, då blir granulariteten lidande och som med allt annat så får man väga och se vad som är relevant baserat på dess context. Finns alltid för och nackdelar och även missbruk av regler och principer som kan försämra istället för att förbättra.
Mvh Fredrik Normén
MVP - MEET - ASPInsiders
http://weblogs.asp.net/fredriknormenSv:Hur tänker ni?
Ex Order.OrderLine.Add där OrderLine är en Lista kommer inte Add kunna ge dig någon bra felhantering då du är låst till den .Net redan har där... Här skulle man mkt väl ex vilja ha ett CannotAddProductException eller liknande för göra det tydligt mot utvecklarna av koden vad man gör för fel etc...
En annan grej med Order.orderline.add är att man även bryter mot OcP (open closed principle) i den anmärkningen att det inte är så lätt att byta ut OrderLine om den i ett läge ex kanske skall kunna hantera både en proidukt och en tjänst... Man bryter även mot command and query separation där man ändrar en annans state genom en annan etc... Sv: Hur tänker ni?
Jag köper båda argumenteringerna vi får väl sätta stämpeln det beror på fall till fall samt utvecklare till utvecklare.Sv:Hur tänker ni?
Sv: Hur tänker ni?
Ofta när vi utvecklar system så väger vi vad som är viktigt och agerar efter det, tex är prestanda eller underhåll viktigast.. robustness eller correctness etc. Det val man gör, gör att man skriver kod på helt olika sätt. Finns ingen Silver Bullet när det gäller lagar, regler eller principer när det kommer till att skriva kod, men det finns vägledning, där olika passar i olika scenarios. Det viktiga är att man känner till dessa, så man väljer rätt för rätt sak, bara en tanke ;)
Mvh Fredrik Normén
MVP - MEET - ASPInsiders
http://weblogs.asp.net/fredriknormenSv:Hur tänker ni?
Exempelvis om man tänker sig att den ursprungliga raden med kod var placerad inuti en metod så här:
public class SomeClass ...
public void addProductToOrder(Order order, Product product) {
order.OrderLines.Add(new OrderLine(product));
}
så bryter alltså ovanstående implementation mot LoD, men för att ändra koden så att den följer LoD så kan man (istället för att enligt alt. 2 lägga till en ny metod i Order-klassen) delegera till en ny metod på detta sätt:
// "Alternativ 3" (LoD compliant):
public void addProductToOrder(Order order, Product product) {
addProductToOrderLines(order.OrderLines, product);
}
private void addProductToOrderLines(IList<OrderLine> orderLines, Product product) {
orderLines.Add(new OrderLine(product));
}
Genom att inte bryta mot LoD så tydliggör man beroenden (i detta konkreta fall ett beroende till list-interfacet) på så sätt att beroendet syns i en metodsignatur.
I detta exempel kan man tycka att ovanstående nya metod som exponerar det beroendet inte tillförde något, men i en större befintlig klass där man inuti många metoder har mycket kod av typen
"a.getB().getC().getD().doSomeStuff()"
så kan det underlätta att göra beroendena mer tydliga för att därmed kunna åtgärda dem med lämpliga refaktoriseringar.
Om man tydligt ser vilka beroenden som finns, och om det finns många beroenden till många typer (klasser/interface) så kan det vara en indikation på att en del av koden borde flyttas till andra klasser för att uppnå förbättrad "low coupling" vilket är önskvärt.
För övrigt är mitt ovanstående "alternativ 3" (som i likhet med alternativ 2 följer LoD) inget jag rekommenderar i detta fall, utan jag är helt klart anhängare till alternativ 2 för att undvika att exponera en referens till en list som är mutable. Jag tycker alltså att man borde tvingas lägga till nya OrderLines via den Order-klass som aggregerar listan (och listan kan f.ö. exponeras som ReadOnlyCollection om ett GUI behöver loopa igenom den).
Visserligen känner jag inte till de konkreta kraven i den aktuella applikationen men sannolikt finns det åtminstone någon typ av begränsning på vad som skall tillåtas stoppa in i en Order med avseende på ordern i sin helhet, t.ex. max totalvikt, max totalpris eller max antal OrderLines.
Denna typen av valideringar passar bra i Order-klassen, genom att applicera den objekt-orienterade principen "Information Expert" (som är ett s.k. GRASP pattern).
Eftersom det är Order som aggregerar informationen om alla OrderItems så är det nämligen lämpligt att låta den klassen utföra valideringarna istället för att exponera listan och validera med extern logik och procedurell programmering.
Om man försöker applicera koncept från Domain-Driven Design så är Order-klassen en het kandidat till att implementeras som en s.k. Aggregate, och en av dess uppgifter är att upprätthålla invarianterna för aggregatet.
(en invariant är f.ö. ett villkor på ett objekt som alltid måste vara sant då objektet är i ett "stabilt" tillstånd då ingen metod exekverar, men villkoret kan tillåtas vara tillfälligt falskt medan en metod exekverar)
Order-klassen skulle alltså kunna ha t.ex. en invariant som säger att totala antalet OrderItems i Order-instansen måste vara mindre än X.
Självklart kan X parametriseras, och t.ex. skickas in till Order-konstruktorn som en max-gräns, men man kan förstås också använda mer generella valideringsmekanismer och t.ex. skicka in en lista av implementationer av ett valideringsstrategi-interface till konstruktorn som Order-klassen sedan kommer att loopa igenom dessa när man försöker lägga till en ny OrderItem.
(De konkreta implementationerna av valideringsinterfacet skulle man f.ö. kunna skicka in m.h.a. ett ramverk för Dependency Injection, så att man kan konfigurera validatorerna utan omkompilering)
Visst skulle man kunna utföra validerigarna även med procedurell kod, t.ex. en metod
"OrderValidationUtility.ValidateOrderItemCollection(order.OrderLines)"
men genom att tillåta direkt modifiering av listan och sedan förvänta sig externt anropad validering, så blir ju valideringen i praktiken valfri, men genom att placera den i Aggregatet (Order-klassen) så blir den alltså tvingande.
Mitt svar på den ursprungliga frågan hur man ska tänka angående alt. 1 eller alt. 2 är alltså att möjligheten att implementera tvingande validering i Order klassen är det främsta argumentet för att använda alternativ 2, d.v.s. "Order.AddLine(product);".
En mycket positiv bieffekt är också att det alternativet också följer LoD (även om det som jag nämnde finns fler sätt att följa LoD).
/ Tomas
Sv: Hur tänker ni?
"Mitt svar på den ursprungliga frågan hur man ska tänka angående alt. 1 eller alt. 2 är alltså att möjligheten att implementera tvingande validering i Order klassen är det främsta argumentet för att använda alternativ 2, d.v.s. "Order.AddLine(product);". "
+1
Vi ska vara medvetna om de för och nackdelar som finns när det gäller olika lagar och principer.. bara för att dom finns betyder inte att dom är Silver bullets. Som Michael T. Nygard nämner i sin bok "Release It!" ur The pragmatic Programmers serien: Det är inte alltid viktig att följa alla principer, i vissa fall måste man undvika dom för att lösa andra viktiga problem.
Mvh Fredrik Normén
MVP - MEET - ASPInsiders
http://weblogs.asp.net/fredriknormenSv: Hur tänker ni?
Skön post. Lite lång, hehe men du fick med många bra tankar... Kul
Allmänt:
Principer, Patterns, Guideline etc alla har en gemensam nämnare de är öppna för förändring och undantag vilket är ottroligt skönt. Vore synd annars, men att de finns där är mest för att försöka höja kvalitén i kod, och för dem som inte känner till att de faktiskt kan göra bättre kod är principerna guld värda att titta på och försöka följa för att enklare faktiskt lära sig de ev undantag som kan finnas...
De kan ses lite som en vägledning till bättre kod.