Använda 'object' för två olika domänobjekt

Jag tror en mappning eller wrappning hade varit en enklare väg att gå i detta fallet.

Ja, det kommer han som sagt inte undan och det försöker vi inte (inte jag ivf ;)) gå kring. Men det handlar ju även om att faktureringsapplikationen ska kunna hantera framtida Order-objekt också - inte enbart de som finns nu. Då är vägen via interfaces vägen att gå.

Det lutar åt att mappa orderraderna men vill bara testa detta först.
Jag får dock: Reference not set to an instance of an object på denna:
HttpContext.Current.Response.Write(_order.OrderItems.Count.ToString());
Men inte om jag byter till Items.

public IList<OrderItem> Items
        {
            get {
                if (_items == null)
                    _items = new List<OrderItem>();
                return _items;
            }
            set { _items = value; }
        }

public IList<IInvoiceableOrderItem> OrderItems
        {
            get
            {
                return Items as IList<IInvoiceableOrderItem>;
            }
        }

Jag får tydligen inte hämta OrderItems på detta sätt!

doggeito skrev:

Jag får tydligen inte hämta OrderItems på detta sätt!

Nej jag tror inte att din typomvandling är korrekt, men eftersom du använder as så kommer du få NULL tillbaka om Items inte är av rätt typ.

Just när det gäller att typomvandla listor, så har jag inte hittat något bra sätt, utan man får helt enkelt mappa varje objekt till en ny lista.

IList<IInvoiceableOrderItem> list = new List<IInvoiceableOrderItem>();
for(int i = 0; i < Items.Count; i++)
   list.Add((IInvoiceableOrderItem)Items[i]);

return list;

Något sådant, inte speciellt snyggt, men jag har faktistk inte hittat något smidigare sätt, men det borde finnas något...

- M

Gladh skrev:

Något sådant, inte speciellt snyggt, men jag har faktistk inte hittat något smidigare sätt, men det borde finnas något...

Ja, verkligen! Det måste ju finnas.

Personligen så tycker jag att det är snyggare att köra en foreach...

foreach (Item item in Items)
{
    list.Add((InvoiceableOrderItem)item);
}

listan bör ju redan vara sorterad osv och är den inte det och det är av intresse så går det ju lösa på annat vis. Vet inte vad ni andra säger men jag tycker helt klart det är snyggare. Dessutom så slipper man stöta på indexoutofrange exceptions osv i koden ;)

doggelito skrev:

Gladh skrev:

Något sådant, inte speciellt snyggt, men jag har faktistk inte hittat något smidigare sätt, men det borde finnas något...

Ja, verkligen! Det måste ju finnas.

Tycker det är onödigt - till och med farligt - att exponera en lista direkt i interfacet. Borde det inte räcka med en IEnumerable, dvs något man kan iterera över?

public interface IInvoiceableOrder
{
     ...
     public [b]IEnumerable<IInvoiceableOrderItem>[/b] Items { get; }
}

public interface IInvoiceableOrderItem
{
     string ArticleNr { get; set; }
}

public class OrderItem : IInvoiceableOrderItem
{
     public int Id { get; set; }
     public string ArticleNr { get; set; }
     ...
}

public class Order : IInvoiceableOrder
{
     public int Id { get; set; }

     private readonly IList<OrderItem> _orderItems = new List<OrderItem>();
     [b]IList<OrderItem> Items { get { return _orderItems; } }[/b]

     [b]IEnumerable<IInvoiceableOrderItem> IInvoiceableOrder.Items[/b]
     { 
          get { return _orderItems.Cast<IInvoiceableOrder>(); } // extension från System.Linq
     }
}

Och... in before the law of Demeter.

Vill man nödvändigtvis göra en IList av en typ T som bara kan innehålla objekt av en subtyp till T kan man göra en wrapper i stil med:

public class EvilList<TSub, TSuper> : IList<TSuper>
    where TSub : TSuper
{
    private readonly IList<TSub> _source;
    public EvilList(IList<TSub> source) { _source = source; }

    public void Add(TSuper obj) { _source.Add((TSub) obj); }

    // etc, delegera alla anrop till _source, casta från TSuper till TSub där det behövs
}

I din Order-klass skulle det då bli

     private readonly IList<OrderItem> _orderItems = new List<OrderItem>();
     [b]private readonly IList<IInvoiceableOrderItem> _invoceableOrderItems = new EvilList<OrderItem, IInvoiceableOrderItem>(_orderItems);
     IList<OrderItem> Items { get { return _orderItems; } }[/b]

     [b]IList<IInvoiceableOrderItem> IInvoiceableOrder.Items[/b]
     { 
          get { return _invoceableOrderItems; } 
     }

... men jag tror man kan hamna i helvetet för det här.

spango skrev:

_orderItems.Cast<IInvoiceableOrder>();

Det ser ju ut som något man kan använda. Cast<T> fungerar det på alla typomvandlingar som mina objekt i listan kan ha? Borde ju göra det...

- M

spango skrev:

Tycker det är onödigt - till och med farligt - att exponera en lista direkt i interfacet

Varför tycker du det är faligare att exponera IList<T> istället för IEnumerable<T>, är det för möjligheten att operera på listan?

- M

Gladh skrev:

Det ser ju ut som något man kan använda. Cast<T> fungerar det på alla typomvandlingar som mina objekt i listan kan ha? Borde ju göra det...

Egendefinierade cast-operatorer verkar inte funka, säger mina tester, men jag kan ha gjort fel. Det är bara "riktiga" cast som funkar.

Gladh skrev:

Varför tycker du det är faligare att exponera IList<T> istället för IEnumerable<T>, är det för möjligheten att operera på listan?

Svar ja. Eftersom listan bara får innehålla OrderItem vill vi inte att någon pajar programmet genom att stoppa in en massa icke-OrderItem-objekt som implementerar IInvoiceableOrderItem, och vi vill inte lura någon att det här är en lista som det är OK att stoppa in vilken IInvoiceableOrderItem som helst i.

spango skrev:

Svar ja. Eftersom listan bara får innehålla OrderItem vill vi inte att någon pajar programmet genom att stoppa in en massa icke-OrderItem-objekt som implementerar IInvoiceableOrderItem, och vi vill inte lura någon att det här är en lista som det är OK att stoppa in vilken IInvoiceableOrderItem som helst i.

Hmm, men är det verkligen så farligt?

Om man implementerar IInvoiceableOrderItem på sin klass, gör det då något om klassen är av helt annan typ än OrderItem?
Jag menar den har ju blivit "godkänd" av interfacet IInvoiceableOrderItem och borde då kunna användas i listan.

doggelito skrev:

Om man implementerar IInvoiceableOrderItem på sin klass, gör det då något om klassen är av helt annan typ än OrderItem?
Jag menar den har ju blivit "godkänd" av interfacet IInvoiceableOrderItem och borde då kunna användas i listan.

Det är det bara du som kan svara på, men eftersom du i Order-klassen ville kunna använda dem som OrderItem antar jag att OrderItem har nåt som inte IInvoiceableOrderItem har? I sådana fall får du en trist överraskning när du tror att alla element i listan är OrderItems (eller om du använder min EvilList-wrapper, när du gör Add).

Du har ju alternativet ReadonlyCollection också.
http://msdn.microsoft.com/en-us/library/ms132474.aspx

Men blir det inte lite väl krångligt nu? ;)

Nu har jag faktiskt skrivit ner på ett ungefär hur jag har tänkt, så jag bara hoppas att du kan applicera något sånt här på din app. Jag har haft lite problem med generics i Mono, så jag har inte kunnat testa koden tyvärr. Error management är upp till dig alltså! ;)

using System;
using System.Collections.Generic;

namespace Application
{
	interface Invoiceable {
		List<Orderable> GetCollection();
		int GetUserID();
	}
	interface Orderable {
		string GetItemName();
	}
	class OrderItem : Orderable {
		private string itemName;
		
		public OrderItem(string itemName) {
			this.itemName = itemName;
		}
		public string ItemName {
			get { return this.itemName; }
			set { this.itemName = value; }
		}
		public string GetItemName() {
			return this.ItemName;
		}
	}
	class OldOrder {
		private int userID;
		private List<OrderItem> items;

		public OldOrder() {
			this.items = new List<OrderItem>();
		}
		
		public int UserId {
			get { return this.userID; }
			set { this.userID = value; }
		}
		public List<OrderItem> ItemCollection {
			get { return this.items; }
			set { this.items = value; }
		}
	}
	class NewOrder : Invoiceable {
		private int user;
		private List<OrderItem> items;
		
		public NewOrder() {
			this.items = new List<OrderItem>();
		}

		public int UserId {
			get { return this.user; }
			set { this.user = value; }
		}
		public List<OrderItem> Items {
			get { return this.items; }
			set { this.items = value; }
		}
		public List<Orderable> GetCollection() {
			List<Orderable> list = new List<Orderable>();

			foreach(OrderItem item in this.items) {
				list.Add((Orderable)item); // Att casta om till Orderable är nödvändigt, tydligen
			}
			return list;
		}
		public int GetUserID() {
			return this.user;
		}
	}
	
	class OrderMapper {
		public static NewOrder MapFromOldOrder(OldOrder oldOrder) {
			NewOrder order = new NewOrder();
			order.UserId = oldOrder.UserId;
			order.Items = oldOrder.ItemCollection;
			return order;
		}
	}
	
	[b]class Invoice {
		public static void SendInvoiceForOrder(Invoiceable order) {
			List<Orderable> items = order.GetCollection();
			Console.WriteLine("Items for user " + order.GetUserID() + ":");
			foreach(Orderable item in items) {
				Console.WriteLine(item.GetItemName());
			}
		}
	}[/b]
	
	public class YourClass {
		public YourClass() {
			OldOrder order1 = new OldOrder();
			order1.UserId = 1;
			List<OrderItem> list = new List<OrderItem>();
			list.Add(new OrderItem("Item 1"));
			list.Add(new OrderItem("Item 2"));
			order1.ItemCollection = list;
			
			NewOrder order2 = OrderMapper.MapFromOldOrder(order1);
			Invoice.SendInvoiceForOrder(order2);
		}
		...
	}
}

Det jag vill visa är att din faktureringsapplikation inte ska behöva bry sig om vilka objekt som skickas till den, så länge de implementerar Invoiceable för ordern samt Orderable för artiklarna kan Invoice hantera dessa.

Red.} Var tvungen att uppdatera på vissa ställen såg jag. ;)
Red.2} Nu fungerar mitt exempel åtminstone i Mono. ;)

Supertack SPiN! :bire
Ska sätta tänderna i koden imorgon! :)

doggelito skrev:

public class Order : IInvoiceableOrder
{
     public int Id { get; set; }
     ...
     private IList<OrderItem> _orderItems;
     public IList<OrderItem> Items
     {
            get {
                if (_orderItems == null)
                    _orderItems = new List<OrderItem>();
                    return _orderItems; }
            set { _orderItems = value; }
        }
}

spango skrev:

Tycker det är onödigt - till och med farligt - att exponera en lista direkt i interfacet

Ja, jag håller med om att det inte brukar vara lämpligt att publikt exponera en direktreferens till ett objekt som kan modifieras (t.ex. en lista) om den har regler som måste upprätthållas.
Finns det verkligen inga regler på ordern som skulle passa bäst att implementera i order-klassen ?
(det är ju trots allt den klassen som är "Information Expert" med avseende på OrderItems, så därför kan den klassen vara en lämplig kandidat att placera valideringskoden i)

Som ett enkelt exempel på en regel som skulle kunna finnas med avseende på kollektionen som helhet (dvs alla OrderItems) så kan man tänka sig ett maxbelopp på den totala summan.

Ett annat exempel skulle kunna vara att man för vissa varor endast får beställa högst ett visst antal av en viss vara
t.ex. vissa billiga specialerbjudanden brukar kunna ha en regel som säger att man får köpa max en eller två stycken...

Ett lite mer avancerat exempel skulle kunna vara en regel som säger att om man har beställt för ett totalt belopp på 1000 kr så får man välja en (men endast en) av tre stycken gratis items, som f.ö. kanske inte ens är till salu utan endast kan få läggas till ordern om villkoret är uppfyllt.
Sådana regler kontrollerar man lämpligen via den aggregerande klassen (Order) via en särskild Add-metod som måste användas, dvs man bör inte få gå runt den valideringen via direktaccess till en private list (vilket man får om man returnerar den med en publik getter, d.v.s. då har man inte så stor nytta av att den är private...)

Nu kan man förstås hävda att denna typ av valideringar kan implementeras i ett senare läge och att man kan låta det vara fritt fram att trycka in vad som helst i en privat kollektion
för att senare validera alla orderItems, men jag tycker man ska validera så snart man kan, och som användare skulle jag bli irriterad om man i en webb-butik får klicka
runt och välja hur många varor som helst och inte förrän långt senare när jag går till varukorgen (och en validering av alla OrderItems triggas)
för att betala få veta att jag har beställt alldeles för mycket.

Om man tänker sig en OrderRepository-implementation (Domain-Driven Design -terminologi, där en repository är interfacet som en klient använder för att persistera en "Aggregate" som är ett annat DDD-koncept vars uppgift är bl.a. att uprätthålla invarianterna för de olika delar, d.v.s Entities och Value objects som ingår i aggregatet) så kan man tänka sig att _försöka_ tvinga utvecklarna av klientkoden att alltid anropa en valideringsmetod innan persisteringen anropas, d.v.s. så här:

OrderRepository orderRepository = ... [instansiera en implementation av OrderRepository-interfacet]
order.validateForInsertion(); // detta måste klientkoden komma ihåg att göra för att undvika otillåtna order att hamna i databasen om man tillåter direktaccess till OrderItems
orderRepository.AddOrder(order)

Ovanstående typ av metod-anrop kan alltså utvecklarna av klienterna tvingas utföra ifall man tillåter direktaccess till OrderItems, men om valideringen hela tiden görs via Order-objektet så kan man säkerställa att invarianterna (med avseende på OrderItems) kan göras kontinuerligt, och därför inte behöver förlita sig på att klientkoden kommer att utföra en validering innan persisteringen blir utförd, t.ex. via ett Repository.

Jag menar alltså att man istället för att exponera en direktreferens till en IList som klienterna fritt kan addera till, så kontrollerar man vad som tillåts adderas genom en särskilt AddOrderItem-metod, och om man behöver iterera igenom alla OrderItems så kan man exponera någon iterator som är immutable, t.ex. en IEnumerator, så här:

    public class Order
    {
        private IList<OrderItem> orderItems = new List<OrderItem>();

        public IEnumerator<OrderItem> GetOrderItems()
        {
            return orderItems.GetEnumerator();
        }

        public void AddOrderItem(OrderItem orderItem)
        {
            // verifiera Order-objektets invariant som säger att den totala orderkostnaden aldrig får överstiga ett maxbelopp:
            if (GetTotalCostForAllCurrentOrderItems() > GetMaximumAllowedTotalCostForTheOrder()) // private hjälpmetoder
            {
                // throwException/returnFalse/addErrorMessageToNotificationObject
            }

            // verifiera att totala antalet av en viss produkt (i en OrderItem) inte överstiger en maxgräns 
            // t.ex. vissa billiga specialerbjudanden brukar kunna ha en regel som säger att man får köpa max en eller två
            if (GetNumberOfSpecfiedProductAlreadyAdded(orderItem) > GetMaximumAllowedNumberOfTheProduct(orderItem)) // TODO: bättre metodnamn... kommer inte på något optimalt just nu...
            {
                // throwException/returnFalse/addErrorMessageToNotificationObject
            }

            // verifiera regeln om att en (men inte fler än en) gratis item endast får läggas till om maxbeloppet är tillräckligt stor
            if (IsOneOfTheFreebieItems(orderItem)) // private hjälpmetod
            {
                if(HasSomeFreebieItemAlreadyBeenAddedToTheOrder())  // private hjälpmetod
                {
                    // inga fler än en gratis item får läggas till
                    // throwException/returnFalse/addErrorMessageToNotificationObject
                }
                if (GetTotalCostForAllCurrentOrderItems() < GetMinimumTotalCostForTheOrderThatQualifiesForFreebieItem()) // private hjälpmetoder
                {
                    // har inte beställt tillräckligt mycket för att få en gratis item
                    // throwException/returnFalse/addErrorMessageToNotificationObject
                }
            }

            // nu är alla invarianter verifierade och då kan vi lägga till en orderItem till kollektionen
            this.orderItems.Add(orderItem);
        }

        // ...
   }

Detta var bara några exempel på valideringar som man skulle kunna tänka sig, och det skulle verkligen förvåna mig om det inte finns några som helst valideringar med avseende på Ordern som helhet, dvs med avseende på alla de OrderItems som ligger i ordern.

För övrigt kan det diskuteras hur valideringen ska utföras, men antagligen vill man inte bara returnera false, utan vill hellre ha ett felmeddelande med information om varför inte en OrderItem kunde adderas.
Ett uppenbart alternativ är förstås att kasta exception, men en annan variant är att tillämpa någon variant på Notification pattern, som helt enkelt är ett object som kan samla upp flera informationsmeddelanden.
Sedan behöver man förstås inte heller hårdkoda valideringarna enligt ovan, utan kan t.ex. definiera ett validerings-interface, vars olika implementationer kan specificeras dynamiskt i runtime via konfigurering (alltså inga nya statiska beroenden som kräver omkompilering av Order-klassen då man lägger till nya regler) t.ex. m.h.a. ett dependency injection ramverk. Mer detaljer om en sådan implementation är dock off-topic för detta inlägg, men min poäng var alltså att illustrera varför det inte är lämpligt att exponera direkta referenser till klienter som direkt kan lägga till nya objekt utan att ta hänsyn till regler som måste uprätthållas. Angående att man ska undvika att exponera referenser kan jag även passa på att tipsa om artikeln Data Access Routines, och på slutet i den PDF-artikeln skriver Martin Fowler att "...any situation where one object makes multiple calls to another’s accessors." vilket är en situation han har skrivit mer om i artikeln GetterEradicator som är en typiskt procedurell programmeringsmodell som brukar appliceras på en s.k. AnemicDomainModel då man alltså har externaliserat logiken till separata objekt som anropar getters på domänobjekten.

/ Tomas