views:

98

answers:

5

Let's say I have a class which has three properties as below.

public class Travel
{
  public int MinAirportArrival { get; set; }
  public int MinFlightTime { get; set; }
  public int TotalTravelTime { get; set; }
}

TotalTravelTime must be at least the sum of MinAirportArrival and MinFlightTime but could also be more in the event there is a stopover or something of the sort.

It is clear to me that I can put logic in the setter for TotalTravelTime.

My question is regarding the changing of MinFlightTime and MinAirportArrival. Is it right to expect that TotalTravelTime be increased first and if not to throw an exception when one of the others will make the sum larger that TotalTravelTime?

What are my other options for controlling this in a reasonable fashion?

Should I just leave this to the object responsible for saving the state to check a valid property on the class? I may have other logic to put in there as well.

EDIT
I am not storing anywhere an amount for the extra time if there is any so this is not just a matter of adding up a few properties. Just to clarify this class is just a contrived example of the issue I am facing but I think it matches the problem pretty well.

+1  A: 

Since TotalTravelTime is derived from the other two, I would not implement a public setter for it, but instead a private one that is updated by the setters for the other two.

Josh Pearce
It is not completely derived. It just has to be a minimum of the sum.
YonahW
+6  A: 

The Framework Design Guidelines recommend that properties should not depend on each other. It should always be possible to set the values of multiple properties in arbitrary order.

Here are the options that is see:

  1. In this specific case:

    public class Travel
    {
      public int MinAirportArrival { get; set; }
      public int MinFlightTime { get; set; }
      public int AdditionalTravelTime { get; set; }
      public int TotalTravelTime
      {
        get { return MinAirportArrival + MinFlightTime + AdditionalTravelTime; }
      }
    }
    

    This takes advantage of the effect that the TotalTravelTime can be reconstructed from the three other values which can be indivually set without dependencies.

  2. A solution for the general case is to accept any value and validate the values when, for example, the instance is sent to storage.

    public class Travel
    {
      public int MinAirportArrival { get; set; }
      public int MinFlightTime { get; set; }
      public int TravelTime { get; set; }
      public void Save()
      {
        // validate TravelTime > MinAirportArrival + MinFlightTime 
      }
    }
    
  3. Another option is to make the properties read-only and provide a method to batch update the values of the properties.

    public class Travel
    {
      public int MinAirportArrival { get; private set; }
      public int MinFlightTime { get; private set; }
      public int TravelTime { get; private set; }
      public void UpdateTimes(
        int minAirportArrival, int minFlightTime, int travelTime)
      {
        // validate travelTime > minAirportArrival + minFlightTime 
        MinAirportArrival = minAirportArrival;
        MinFlightTime = minFlightTime;
        TravelTime = travelTime;
      }
    }
    
  4. Alternatively, you can make Travel objects immutable and use a constructor, factory method or mutable builder object to create instances.

    Constructor:

    public class Travel
    {
      public Travel(int minAirportArrival, int minFlightTime, int travelTime)
      {
        // validate travelTime > minAirportArrival + minFlightTime 
      }
      public int MinAirportArrival { get; }
      public int MinFlightTime { get; }
      public int TravelTime { get; }
    }
    

    Factory method:

    public class Travel
    {
      public static Travel CreateTravel(
        int minAirportArrival, int minFlightTime, int travelTime)
      {
        // validate travelTime > minAirportArrival + minFlightTime 
        return new Travel(minAirportArrival, minFlightTime, travelTime);
      }
      private Travel(int minAirportArrival, int minFlightTime, int travelTime);
      public int MinAirportArrival { get; }
      public int MinFlightTime { get; }
      public int TravelTime { get; }
    }
    

    Builder class:

    public class TravelBuilder
    {
      public int MinAirportArrival { get; set; }
      public int MinFlightTime { get; set; }
      public int TravelTime { get; set; }
      public Travel BuildTravel()
      {
        // validate TravelTime > MinAirportArrival + MinFlightTime 
        return new Travel(MinAirportArrival, MinFlightTime, TravelTime);
      }
    }
    

    Examples of all three options can be found in the .NET framework.

dtb
the AdditionalTravelTime is could be a variety of things and is not being stored.
YonahW
But the TotalTravelTime is stored in your code. My suggestion is, instead of storing the triple `(MinAirportArrival, MinFlightTime, TotalTravelTime)` to store `(MinAirportArrival, MinFlightTime, AdditionalTravelTime)` which represents exactly the same data, just with the difference to the known `TotalTravelTime` instead of the `TotalTravelTime` itself.
dtb
A: 

I think its best to put your custom validation logic into a private helper method and then call this method from each property setter like this:

public class Travel
{
    private int minAirportArrival;
    private int minFlightTime;
    private int additionalTravelTime;

    public int MinAirportArrival
    { 
        get
        {
            return this.minAirportArrival;
        }
        set
        {
            ThrowOnImpossibleTimes(value, this.minFlightTime, this.additionalTravelTime);
            this.minAirportArrival = value;
        }
    }

    // other properties...

    private static void ThrowOnImpossibleTimes(int minAirportArrival, int minFlightTime, int additionalTravelTime)
    {
        // do check and eventually throw...
    }
}
Thomas Weller
A: 

One way of doing it is to implement a validation method checking the condition. The method would be called from the setter of every property and throw an exception if the condition is not met

mfeingold
+1  A: 

To allow the properties to be set in any order, but retain an established validation API (used automatically by some APIs, including DataGridView and ASP.NET MVC validation), one option is IDataErrorInfo. This isn't the simplest interface to implement, but is workable (below). You can generalise your validation as something like:

IDataErrorInfo dei = obj as IDataErrorInfo;
if(dei != null) {
    string err = dei.Error;
    if(!string.IsNullOrEmpty(err)) throw new SomeTypeOfException(err);
}

Here's an implemention:

public class Travel : IDataErrorInfo
{
    public int MinAirportArrival { get; set; }
    public int MinFlightTime { get; set; }
    public int TotalTravelTime { get; set; }
    string IDataErrorInfo.this[string property] {
        get {
            switch (property) {
                case "TotalTravelTime":
                    if (TotalTravelTime < MinAirportArrival + MinFlightTime) {
                        return "not enough time blah";
                    }
                    break;
            }
            return "";
        }
    }
    string IDataErrorInfo.Error {
        get {
            StringBuilder sb = new StringBuilder();
            AppendError(ref sb, "MinAirportArrival");
            AppendError(ref sb, "MinFlightTime");
            AppendError(ref sb, "TotalTravelTime");
            return sb == null ? "" : sb.ToString();
        }
    }
    void AppendError(ref StringBuilder builder, string propertyName) {
        string error = ((IDataErrorInfo)this)[propertyName];
        if (!string.IsNullOrEmpty(error)) {
            if (builder == null) {
                builder = new StringBuilder(error);
            } else {
                builder.Append(error);
            }
        }
    }
}
Marc Gravell