6

I have a class with three properties:

class PriceCondition
{
    public Product SalesCode {...}
    public ControlDate Condition {...}
    public PriceDetail Pricing {...}
}

Any instance of PriceCondition can only have EITHER a SalesCode or a Condition. If Condition is chosen, then Pricing is required, but that's not pertinent to this discussion.

Being a relatively poor programmer, I initially tried the following:

    public Product SalesCode
    {
        get { return _salesCode; }
        set 
        {
            this.Condition = null;
            _salesCode = value; 
        }
    }

    public ControlDate Condition
    {
        get { return _cond; }
        set 
        {
            this.SalesCode = null;
            _cond = value; 
        }
    }

In hindsight, it's obvious why that created a stack overflow. Searching for the right way, I found this SO answer about XORing a List, but I can't figure out how to apply that to what I'm trying to do, since Except is an IEnumerable method, and I'm not using a List<T> or anything similar.

How can I ensure that only one of the properties is set at any time? Can I pass in a var CodeOrCondition as a parameter in the constructor, use typeof to figure out what it is, then assign it appropriately? I only sort of understand what I just said, so having some foreknowledge of whether that's going to work or not would be helpful before I set out to write the code.

Update:

After excellent help found in the answer(s), I ended up with this:

public class PriceCondition
{
    #region Constructor
    /// <summary>Create an empty PriceCondition.
    /// </summary>
    /// <remarks>An empty constructor is required for EntityFramework.</remarks>
    public PriceCondition();

    /// <summary>Create a PriceCondition that uses a Sales Code.
    /// </summary>
    /// <param name="salesCode">The Product to use.</param>
    public PriceCondition(Product salesCode)
    {
        SalesCode = salesCode;
        Condition = null;
        Pricing = null;
    }

    /// <summary>Create a PriceCondition that uses a DateControl and Price (e.g. "0D7")
    /// </summary>
    /// <param name="dateControl">The DateControl Condition to use.</param>
    /// <param name="price">The PriceDetail to use.</param>
    public PriceCondition(Condition dateControl, PriceDetail price)
    {
        SalesCode = null;
        Condition = dateControl;
        Pricing = price;
    }
    #endregion
....
}

Second Update:

EntityFramework got in my way. I knew that it required empty constructors, but didn't realize the depth of why. We're finding that using private set is keeping EF from populating objects from the database. At least that's what it looks like. We're persisting the data in our DBInitializer, but the information isn't getting loaded into PriceCondition when we try to use it. The easy answer seemed to be putting the setters back to a standard backing-store method and relying on the business logic layer to never set a SalesCode and a ControlDate on the same PriceCondition. But now that's not working either. We'll bang away at it more, but any suggestions would be really appreciated.

Community
  • 1
  • 1
J.D. Ray
  • 697
  • 1
  • 8
  • 22
  • 3
    why can't you just set the `_cond` and `_salesCode` fields directly in your property setters? – rossipedia Mar 31 '15 at 19:42
  • 1
    This seems like poor modeling. You should make two classes that inherit a common base, each containing only the properties they need. Use `is` or `as` to figure out which type it is then later. – David Pfeffer Mar 31 '15 at 19:45
  • 2
    Properties *usually* don't cause side effects like that. I would probably go with methods instead (`SetSalesCode`, `SetCondition`). – Joe Enos Mar 31 '15 at 19:46
  • Don't use public property setters to enforce business logic. Seems like your class could be a simple immutable value object, with read-only fields/properties and mutation methods that are appropriately named and return a new modified immutable instance. – Alex Mar 31 '15 at 19:49

3 Answers3

7

Set the backing variables instead of the properties:

public Product SalesCode
{
    get { return _salesCode; }
    set 
    {
        _cond = null;
        _salesCode = value; 
    }
}

public ControlDate Condition
{
    get { return _cond; }
    set 
    {
        _salesCode = null;
        _cond = value; 
    }
}

Another approach that I prefer personally is to make an immutable object, and provide constructors for the possible configurations:

class PriceCondition {

  public Product SalesCode { get; private set; }
  public ControlDate Condition { get; private set; }
  public PriceDetail Pricing { get; private set; }

  public PriceCondition(Product salesCode) {
    SalesCode = salesCode;
    Condition = null;
    Pricing = null;
  }

  public PriceCondition(ControlDate condition, PriceDetail pricing) {
    SalesCode = null;
    Condition = condition;
    Pricing = pricing;
  }

}

(You should validate the parameters in the constructors also, but this shows the principle.)

Guffa
  • 687,336
  • 108
  • 737
  • 1,005
  • I went with the second approach. Since I'm using EntityFramework, I also had to have a parameterless constructor, so I added that, then referred everything to backing store fields. There's a little more work to do, but I'm underway. – J.D. Ray Mar 31 '15 at 21:29
0

Just one small change to your attempt will prevent the stack overflow:

public Product SalesCode
{
    get { return _salesCode; }
    set 
    {

        if (value != null)

            this.Condition = null;
        _salesCode = value; 
    }
}
Ben Voigt
  • 277,958
  • 43
  • 419
  • 720
0

I can think of something like this:

    public class PriceCondition
{
    private Product m_Product;
    private ControlDate m_ControlDate;

    public PriceCondition(Product product)
    {
        m_Product = product;
    }

    public PriceCondition(ControlDate controlDate)
    {
        m_ControlDate = controlDate;
    }

    public Product SalesCode
    {
        get { return m_product; }
    }

    public ControlDate Condition
    {
        get { return m_ControlDate; }
    }
}

The two constructors will always guarantee that one of the properties is not set (it's in fact null). With the two read-only properties you can always see which one is set (and one of them will be set, because of the constructors)

Dimitar Tsonev
  • 3,711
  • 5
  • 40
  • 70