0

I am not a very experienced programmer. So I ask.

The field in question is the "Amount" field. I store an amount only in a couple levels of the derived class.

The code is a slice of the POCO classes that are making up my data model, the datalayer is Entity framework, code first.

I mention that because the reason I did the amount property like I did is because when I placed the Amount just in a derived class or two and not in the Navigation class, the table in the database would have an extra field for each class with an Amount property, like (Amount, Amount1, Amount2). I did not like that.

So after screwing around with the code for awhile I discovered that...

Placing the Amount property in the navigation class and overriding it in the classes that implemented Amount gave me a table with just one "Amount" field that was Null, unless it was one of the classes that used amount in which case it had a value, which was just what I wanted. I liked that.

So the question is, is the way the Amount property is handled completely proper or have some downside?

public class Navigation
{
    int Id { get; set; }
    string Title { get; set; }
    string Description { get; set; }
    ObservableCollection<Navigation> Children { get; set; }

    public virtual decimal? Amount
    {
        get { return null; }
        set { value = null; }
    }
}
public class C1 : Navigation { }
public class C2 : Navigation { }
public class C3 : Navigation
{
    private decimal _amount;
    public override decimal? Amount
    {
        get { return _amount; }
        set { if (value != null) _amount = (decimal)value; }
    }
}
public class C4 : Navigation
{
    private decimal _amount;
    public override decimal? Amount
    {
        get { return _amount; }
        set { if (value != null) _amount = (decimal)value; }
    }
}
  • 1
    Let me answer your question with a question. How would you like it if your TV's remote had buttons that never do anything...just because the TV makers planned on building a TV that they would work on? – cHao Oct 4 '14 at 3:08
  • I get that concept. Just to clarify, The Amount property should not be a virtual property in the navigation class but simply a property in the c3 and c4 classes? – Jon Oct 4 '14 at 3:49
  • @Jon Do you ever create instances of Navigation itself? – Dan1701 Oct 4 '14 at 4:14
  • Yes. One in the DB init looks like this: foreach (Navigation navigation in context.Navigations) { navigation.Lein = -1; navigation.Overflow = 1; navigation.Amount = 1; } – Jon Oct 4 '14 at 4:26
  • 2
    @cHao ....my remote does have those buttons.... – Jimmy Hoffa Oct 4 '14 at 5:44
2

It is a not good practice because people expect something when they see the property in the class. Imagine if someone writes:

var n = new Navigation (...);
n.Amount = 123.45;
......
Console.WriteLine(n.Amount); //null? why???

A better approach would be to at least warn them:

public virtual decimal Amount{
    get { return new NotSupportedException("Amount is not supported in this class."); }
    set { //same as above }
}

But now you'd get the C# practice warning: "properties should behave like fields and should never throw exceptions".

Perhaps you can add a Type field, then throw away inheritance altogether. But I'm not sure if this is appropriate for your scenario (maybe you have lots of extra fields in derived classes).


Of course, if you're not writing a library that will be used by lots of other programmers, there's nothing wrong to bend common practices. Be flexible.

Do you find it confusing to yourself? If it works and you're comfortable with it, go ahead.


What would I do? This is one of the down sides of Entity Framework, which I don't really like anyway. To me, it brings more problems than it solves. I'd just create tables in the DB, create the classes as you wish in C#, then write some helper methods to map them. If all you do is SELECT and INSERT, it's not as hard as you may think. It's outside the scope of this site, but I guarantee you it's around ~50 lines (I've done it).

  • I love EF, and yes it is a source of headaches. The way I did this was not confusing to me, it solved an entity problem, however it did not feel right. But making it right reintroduced my original problem. I thought this problem was an entity problem, so I posted in stack overflow. Turns out that it was in the inheritance to fix it. stackoverflow.com/questions/26191093/… – Jon Oct 4 '14 at 20:04
  • I was more of thinking like, you'd need the base class to establish some common functionality. Yea removing inheritance works as well if you don't need it. – kevin Oct 4 '14 at 20:08
0

One thing you can do if you want to add a property to only a few of the subclasses is to use another interface. In your example: Navigation -> AmountEnabledNavigation -> C3, but Navigation -> C1. In the code that handles the objects, e.g. adds the amounts, you can simply check if the object inherits from AmountEnabledNavigation and then decide based on that.

This also prevents later issues like exceptions when accessing the objects. Although these can be caught and handled, they slow down code considerably.

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.