2
\$\begingroup\$

I have a class library in which I'm using generics as below. I'm not sure if it's an improper use or not. So aside from the fact that it works and everything "depends", id like some specific critique on how this usage might fall apart. In particular In wondering about the shadowing of non generic properties with generic counterparts. Is this an accepted practice?

 public interface IPickOrder
    {
        IPickOrderHeader Header { get; }

        ERPListBase<IPickOrderLine> Lines { get; }
    }



 public interface IPickOrder<TPickOrderHeader, TPickOrderLine> : IPickOrder
        where TPickOrderHeader : class, IPickOrderHeader
        where TPickOrderLine : class, IPickOrderLine
    {
        new TPickOrderHeader Header { get; }

       new ERPListBase<TPickOrderLine> Lines { get; }
    }




 public class PickOrder : IPickOrder
    {
        public IPickOrderHeader Header { get; protected set; }

        public ERPListBase<IPickOrderLine> Lines { get; protected set; }

        public PickOrder() { }
    }



   public class PickOrder<TPickOrderHeader, TPickOrderLine> : PickOrder, IPickOrder<TPickOrderHeader, TPickOrderLine>
        where TPickOrderHeader : class, IPickOrderHeader, new()
        where TPickOrderLine : class, IPickOrderLine, new()
    {
        public new TPickOrderHeader Header
        {
            get { return base.Header as TPickOrderHeader; }
            protected set { base.Header = value; }
        }

        public new ERPListBase<TPickOrderLine> Lines { get; protected set; }

        /// <summary>
        /// 
        /// </summary>
        public PickOrder() : base() { }
    } 
\$\endgroup\$
  • 1
    \$\begingroup\$ As you say, it "depends" - in this case, it depends on the purpose of these types. Why do you need to have both generic and non-generic IPickOrder and PickOrder? Perhaps try to write more about what are you trying to achieve with these types - how are they going to be used? \$\endgroup\$ – Nikola Anusev Jul 12 '12 at 8:07
  • \$\begingroup\$ My reasons are more due to preference than need. Non generic means I can sling very simple types around in the core of the program re: work flow and routing. Then also having the generic types means at the periphery I can have strongly type objects so i do not have to recast everywhere i need more specific functionality... which at the periphery is often tho in the core assembly almost never... just a simple type interface will do. I also find that if i leave out a non generic type at the core, I can often get "type trapped" at the periphery. So at the core i like a very simple model. \$\endgroup\$ – rism Jul 12 '12 at 8:14
  • 4
    \$\begingroup\$ just a quick note - when I see myself using "new" to hide members from a base class I take a step back and rethink the approach... \$\endgroup\$ – jl. Jul 12 '12 at 8:43
  • \$\begingroup\$ If you have only instances of the PickOrder<T> class, then you only use the non-generic PickOrder as a conceptual 'interface' class. In that case, make PickOrder an abstract class. \$\endgroup\$ – Aphelion Jul 12 '12 at 9:25
2
\$\begingroup\$

I feel you on the "My reasons are more due to preference than need." comment but the inheritance from PickOrder and public new ERPListBase<TPickOrderLine> Lines { get; protected set; } will confuse somebody best case and really mess things up worst case. While it may still give you that feeling of impending doom here are some suggestions to make this safer while keeping the look and feel you prefer.

Consider implementing the property like so (also for Header)

public new ERPListBase<TPickOrderLine> Lines {
    get {
        // note that you can't control base.Lines so some values may not be of TPickOrderLine
        return new ERPListBase<TPickOrderLine>(base.Lines.OfType<TPickOrderLine>());
    }
    protected set { base.Lines = new new ERPListBase<IPickOrderLine>(value.Cast<IPickOrderLine>()); }
}

Consider not reusing the base class and using explicit interface implementation:

public class PickOrder<TPickOrderHeader, TPickOrderLine> :
    IPickOrder<TPickOrderHeader, TPickOrderLine>
    where TPickOrderHeader : class, IPickOrderHeader, new()
    where TPickOrderLine : class, IPickOrderLine, new()
{
    public TPickOrderHeader Header { get; protected set; }

    IPickOrderHeader IPickOrder.Header { get { return Header; } }

    public ERPListBase<TPickOrderLine> Lines { get; protected set; }

    ERPListBase<IPickOrderLine> IPickOrder.Lines { get { return new ERPListBase<IPickOrderLine>(Lines.Cast<IPickOrderLine>()); } }
}

Whatever you do, avoid storing the same data twice as you may be doing (depending on use) in your current code.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Also be careful not to confuse new with override. \$\endgroup\$ – Dandy Mar 25 '13 at 18:32
  • \$\begingroup\$ All great points especially re: not reusing the base class. 9 months later now and I hate that code my. I've been doing alot more TDD and if nothing else that will teach one the merits of simplicity composition over inheritance etc. The comments now re: if I have to new / shadow I should rethink the architecture are also very valid. Why have a base class if I have to shadow the thing so much it's not even the same class? I think i kinda latched onto to something new (for me) and then proceeded to overuse it everywhere. Each tool is just a tool and each tool has a time & place for it's use. \$\endgroup\$ – rism Mar 26 '13 at 21:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.