views:

480

answers:

8

Hi guys

I can't figure out why the following wont work, any ideas?? public interface IFieldSimpleItem { }

public interface IFieldNormalItem : IFieldSimpleItem
{ }

public class Person
{
    public virtual T Create<T>()
        where T : IFieldSimpleItem
    {
        return default(T);
    }
}

public class Bose : Person
{
    public override T Create<T>()
        where T : IFieldNormalItem //This is where the error is
    {
        return default(T);
    } 
}

The reason why I am doing this is due to the fact that if a developer inherits from Bose, Bose relies on the instance being creating being at least of IFieldNormalItem. Whereas the below only relies on it being IFieldSimpleItem but the above should force it to be at least IFieldNormalItem.

public class Person
{
    public virtual IFieldSimpleItem Create() 
    {
        return null;
    }
}

public class Bose : Person
{
    public override IFieldSimpleItem Create()  
    {
        return null;
    } 
}

Cheers Anthony

+1  A: 

I think the problem is that you override a previously defined method. So effectively you try to change the definition of the method, which is not permitted. Your only choice is to either create a new method, e.g.

public class Bose : Person
{
    public virtual T CreateNormal<T>()
        where T : IFieldNormalItem //This is where the error is
    {
        return default(T);
    } 
}

or require a normal field on the Person class, or do the validation dynamically.

Grzenio
I would have thought that it would be ok because I am making the definition stronger not weaker.
vdhant
A: 

What about this:

public interface IFieldNormalItem : IFieldSimpleItem
{ }

public class Person<T> where T : IFieldSimpleItem
{
    public virtual T Create()
    {
        return default(T);
    }
}

Now you can have Person<IFieldSimpleItem> (corresponds to Person) or Person<IFieldNormalItem> (corresponds to Bose).

tvanfosson
Is it right to make the class generic, to create a generic method?
yapiskan
I would consider this a case of "This is a person who has simple fields" vs "This is a person who has normal fields". In this case you are actually differentiating the person and making the class generic would be acceptable.
tvanfosson
unfortunately yapiskan is right. In this case I can't make the whole class a generic (i.e. Person<T>) because of some casting that I have to do.
vdhant
A: 

The code below is enough to override. The type T is already indicated as need to be implemented by IFieldSimpleItem in base class Person.

public class Bose : Person
{
    public override T Create<T>()
        // where T : IFieldNormalItem // You don't need this line.
    {
        return default(T);
    } 
}

EDIT : I totally got the question wrong so the code above won't solve this case. The only thing you have to do is; not to override the Create method by "override" but "virtual".

public class Bose : Person
{
    public virtual T Create<T>()
        where T : IFieldNormalItem
    {
        return default(T);
    } 
}
yapiskan
But that would only enforce the usage of at least IFieldSimpleItem by the create. The problem is if something inherits from Bose and overrides the create with something that returns an implmentation of IFieldSimpleItem it will error out because bose needs it to be at least IFieldNormalItem.
vdhant
I got your question wrong. By the way I updated my answer.
yapiskan
+1  A: 

It seems you can not change the method's definition but can you make your classes generic instead of the Create Method?

public class Person<T> where T : IFieldSimpleItem
{
    public virtual T Create()
    {
        return default(T);
    }
}

public class Bose<T> : Person<T> where T : IFieldNormalItem
{
    public override T Create()
    {
        return default(T);
    } 
}
bruno conde
unfortunately I can't do this. I have just spent ages removing the style of generics from my code. In this case I can't make the whole class a generic (i.e. Person<T>) because of some casting that I have to do.
vdhant
A: 

The simplest example is that this breaks polymorphism. If you had a collection of Person, where one or more of those items is of the type Bose, this would crash as soon as it hits a Bose.

Person[] people;
[...initialize this somewhere...]

foreach(Person p in people)
  p.Create<IFieldSimpleItem>();
Brent Rockwood
Why would it crash?
yapiskan
+1  A: 

I'm pretty sure you're out of luck as far as using the compiler and generics to save you some runtime checks. You can't override something that doesn't already exist, and you can't have different return types to the same methods.

I can't say I completely understand your motivation, but it has technical merit.

My first attempt was using the base class having a Non-Virtual public interface, and then having another protected virtual method CheckCreatedType that would allow anything in the chain to inspect the type before the base class Create was called.

public class A
{
    public IFieldSimpleItem Create()
    {
        IFieldSimpleItem created = InternalCreate();
        CheckCreatedType(created);
        return created;
    }

    protected virtual IFieldSimpleItem InternalCreate()
    {
        return new SimpleImpl();
    }
    protected virtual void CheckCreatedType(IFieldSimpleItem item)
    { 
        // base class doesn't care. compiler guarantees IFieldSimpleItem
    }
}
public class B : A
{
    protected override IFieldSimpleItem InternalCreate()
    {
        // does not call base class.
        return new NormalImpl();
    }
    protected override void CheckCreatedType(IFieldSimpleItem item)
    {
        base.CheckCreatedType(item);
        if (!(item is IFieldNormalItem))
            throw new Exception("I need a normal item.");

    }
}

The following sticks in runtime checking at the base class. The unresolvable issue is you still have to rely on the base class method being called. A misbehaving subclass can break all checks by not calling base.CheckCreatedType(item).

The alternatives are you hardcode all the checks for all subclasses inside the base class (bad), or otherwise externalize the checking.

Attempt 2: (Sub)Classes register the checks they need.

public class A
{
    public IFieldSimpleItem Create()
    {
        IFieldSimpleItem created = InternalCreate();
        CheckCreatedType(created);
        return created;
    }

    protected virtual IFieldSimpleItem InternalCreate()
    {
        return new SimpleImpl();
    }

    private void CheckCreatedType(IFieldSimpleItem item)
    {
        Type inspect = this.GetType();
        bool keepgoing = true;
        while (keepgoing)
        {
            string name = inspect.FullName;
            if (CheckDelegateMethods.ContainsKey(name))
            {
                var checkDelegate = CheckDelegateMethods[name];
                if (!checkDelegate(item))
                    throw new Exception("failed check");
            }
            if (inspect == typeof(A))
            {
                keepgoing = false;
            }
            else
            {
                inspect = inspect.BaseType;
            }
        }
    }

    private static Dictionary<string,Func<IFieldSimpleItem,bool>> CheckDelegateMethods = new Dictionary<string,Func<IFieldSimpleItem,bool>>();
    protected static void RegisterCheckOnType(string name, Func<IFieldSimpleItem,bool> checkMethod )
    {
        CheckDelegateMethods.Add(name, checkMethod);
    }
}
public class B : A
{
    static B()
    {
        RegisterCheckOnType(typeof(B).FullName, o => o is IFieldNormalItem);
    }

    protected override IFieldSimpleItem InternalCreate()
    {
        // does not call base class.
        return new NormalImpl();
    }
}

The check is done by the subclass registering a delegate to invoke in base class, but without the base class knowing all the rules upfront. Notice too that it's still the Non-Virtual public interface which allows the base class to check the results before returning them.

I'm assuming that it's a developer error that you're trying to catch. If it's applicable, you can adorn the runtime check method with System.Diagnostics.Conditional("DEBUG")], allowing the Release version to skip the checks.

My knowledge of generics isn't perfect, so maybe this is unnecessary. However the checks here don't have to be for type alone: this could be adapted for other uses. e.g. the delegate passed in Register.. doesn't have to just check the reference is a specific type'

* Note that it's probably not good to create the dictionary on the type name as written above; this working is a little simplistic in order to illustrate the mechanism used.

Robert Paulson
Thanks, even though this check is done at run time I think that this is the closest I am going to get. Cheers
vdhant
A: 

Changing the generic constraint changes the method signature which is not allowed if you're overriding a virtual.

I think you may need to split the Create method into a separate class:

public interface IFieldSimpleItem { }

public interface IFieldNormalItem : IFieldSimpleItem{ }

public interface IFieldCreator<TField, TPerson> where TField : IFieldSimpleItem where TPerson : Person
{
    TField Create(TPerson person);
}

public class Person
{
}

public class Bose : Person
{
}

public class PersonFieldCreator : IFieldCreator<IFieldSimpleItem, Person> 
{
    public IFieldSimpleItem Create(Person person) { return null; }
}

public class BoseFieldCreator : IFieldCreator<IFieldNormalItem, Bose>
{
    public IFieldNormalItem Create(Bose person) { return null; }
}
Andrew Kennan
The problem is @vdhant has a constraint that he/she wants to ensure subclasses of Bose Create a type that is at least a IFieldNormalItem
Robert Paulson
In this example BoseFieldCreator would work for classes derived from Bose.
Andrew Kennan
+1  A: 

That's not allowed because it violates Liskov Substitution Principle.

Let's say you have another interface:

public interface IFieldSuperItem : IFieldSimpleItem

You then might do this

Person p = new Boss();
p.Create<IFieldSuperItem>();

The call in second line, while compatible with the definition of Create in Person but obviously not compatible to that defined in Boss (which only work with IFieldNormalItem and its subclass).

Buu Nguyen