8

I work in a system that can represent a "shipping estimate" in two ways:

  1. A specific date: The item is guaranteed to ship at that date
  2. A day interval: The item will be shipped "X to Y" days from today

The information on the model is semantically the same, it is "the shipping estimate". When I get the information about the shipping estimate from the system, I can tell whether the estimate is of the first form or the second form.

The current model for this is similar to the following:

class EstimattedShippingDateDetails
{
    DateTime? EstimattedShippingDate {get; set;}
    Range? EstimattedShippingDayRange {get; set;}
}

Range is a simple class to wrap a "beginning -> end" of integers, somewhat like this:

struct Range
{
    int Start {get; set}
    int End {get; set}

    public override ToString()
    {
        return String.Format("{0} - {1}", Start, End);
    }
}

I don't like this approach because only one of the properties on the estimate model will ever be populated, and I need to test for null on one of them and assume the other one has the data.

Each of the properties is shown differently to the user but on the same spot on the UI, using a custom MVC DisplayTemplate, where the current switching logic resides:

@Model EstimattedShippingDateDetails

@if (Model.EstimattedShippingDate.HasValue)
{
    Html.DisplayFor(m => Model.EstimattedShippingDate)
}
else
{
    Html.DisplayFor(m => Model.EstimattedShippingDayRange)
}

How could I model this to make it more representative of the actual requirement while still keeping the display logic simple in an MVC application?

I thought about using an interface and two implementations, one for each "type" of estimate, but I can't seem to wrap my head around a common interface for both. If I create an interface without any members, then I can't access the data in a unified way and it is a bad design IMHO. I also wanted to keep the viewmodel as simple as possible. I'd get a "correct by construction" code with this approach though, since there wouldn't need to be any nullables anymore: each implementation would have a non nullable property, either a DateTime or a Range.

I also considered only using a single Range and when situation #1 happens, just use the same DateTime for both Start and End, but this would add complications on how to emit the values to the UI as I would then have to detect if it's a static or interval by comparing the values and properly format the range to either be displayed as a single date or a formatted interval.

It seems that what I need is a concept similar to Typescript's unions: basically a property that can be any of two types. No such a thing exists natively in C# of course (only dynamic would be close to that).

migrated from codereview.stackexchange.com Mar 10 '16 at 1:05

This question came from our site for peer programmer code reviews.

  • This is more of a conceptual "How can I …?" question than a request for open-ended critique. Migrating this question to Programmers. – 200_success Mar 10 '16 at 1:05
  • @200_success Sorry about that. I thought this would be appropriate for codereview but you are correct, it does fit better here. Thanks for the help ;) – julealgon Mar 10 '16 at 1:36
  • Is the formatting set in stone, or could you for example format interval as "will be shipped between $date1 and $date2"? – svick Mar 10 '16 at 2:04
  • @svick Unfortunately it is set in stone for now. – julealgon Mar 10 '16 at 2:12
17

Use a date range (i.e. two dates) for all shipping estimates.

For a single date, make X and Y equal.

  • I mentioned this in my own question, can you elaborate on the drawbacks I raised for using this approach? Note that the interval scenario is a fixed day interval and not a full date interval as well. – julealgon Mar 10 '16 at 2:46
  • 2
    Populating both fields always should eliminate your objections to null values. What do you mean by "fixed day interval and not a full day interval?" – Robert Harvey Mar 10 '16 at 2:54
  • The data I get from the server is either a DateTime object, representing the expected shipping date, or two integer values with the minimum and maximum days from today that the item will be shipped. If I standardize on dates, I'll have to convert those integers into properly constructed DateTimes, which would raise the complexity quite a bit due to everything that would need to be taken into account, like client vs server dates, UTC, daylight saving time differences etc. I'd like to avoid creating this kind of complexity at this point. – julealgon Mar 10 '16 at 3:03
  • Good point on the possibility of never having null values though, that would be pretty nice. – julealgon Mar 10 '16 at 3:04
  • What's wrong with the way you're doing it now, using your switch in the MVC template? – Robert Harvey Mar 10 '16 at 3:05
2

You can model this using encapsulation.

Let the class have 2 constructors, one for the single date and another for the date range.

In the ToString() method determine the 'state' of the class and generate the appropriate formatted string.

Add other methods as appropriate for your other needs.

  • That would work well on a simpler system, but notice that I need to properly display this using MVC views, and that the Range type could be used in other places on the system and in that case be displayed the same way. That's why I need to centralize the Range display into a DisplayTemplate that can be reused. If I use the ToString method to encapsulate that, I'm losing a lot of flexibility that views provide. – julealgon Mar 10 '16 at 1:32
  • Also, how do you propose I know the "state" of the object? A local enum variable of sorts? A boolean? I suppose it would be set to different values depending on which constructor was called right? To make this work, I'd also probably need to make the class immutable as well, or I may get into problems if someone sets the other value etc. What do I do if one uses the single date constructor and tries to access the Range property too? Would I throw an exception in that case or just return null? You see, the usability of this is still not ideal. – julealgon Mar 10 '16 at 1:35
  • Yes, essentially the class would be immutable with the 2 dates being readonly and assigned in the constructors. The state could be ascertained by a bool and/or having the same date for representing the single date. Your others member functions could use the state appropriately and you could additionally expose the state, if necessary. Sorry cannot be more specific because I do not know all the functionality you need to support. – hocho Mar 10 '16 at 1:54
  • What @hocho is saying is that all delivery dates can be modeled as ranges, it is just that some ranges have the same start and end date. This seems pretty sensible. Client code using Ranges should be prepared to deal with a start and end date of the same date, and in that case perhaps print different information (i.e. single date vs. date range) in the email. – Erik Eidt Mar 10 '16 at 2:00
  • down vote for ..ToString() [to] determine the state. ToString() should just "report" the state. State is determined in constructors, property setters, and so forth. And, I'm just not seeing anything in the OP that suggests there is or should be a "summary state" – radarbob Mar 13 '16 at 22:54
1

This is a fairly common problem, Nullables for example solve the common place issue of endDates for things which havent ended.

But I think you have chosen a bad example.

With your exact case of two dates, a date range which starts in the morning and ends in the evening seems to be the perfect solution. Or perhpas a start date and an integer number of extra days it might be?

However, lets consider a more difficult case. I have two types of delivery, post and pickup from shop. These are obvs much more different, the shop needs the name and address, the post has a cost, maybey a number of delivery options, tracking codes etc.

The standard approach is to look for the common things which make both of these 'delivery options' and to put those in a base class. The subclass the two specific cases with the extra details they have/need.

In almost all cases you will have at least an Id, a Type and a Description common to both types. So:

public class DeliveryOption 
{
     Public string Id;
     Public typeEnum Type;
     public string Description;
}


Public class Collection : DeliveryOption
{
     Public string ShopName;
}

Public class Post : DeliveryOption
{
     Public DateTime EstDelivery;
}
  • ...a date range which starts in the morning and ends in the evening seems to be the perfect solution. Just use the DataTime.Date property and don't worry about time at all. – radarbob Mar 13 '16 at 22:14
1

"start" and "end" are not DateTime, They are Offsets

How could I model this to make it more representative of the actual requirement while still keeping the display logic simple in an MVC application?

"start" and "end" are offsets to the EstimatedShipDate. They are not DateTimes themselves. This better describes what is happening and it will dramatically reduce the complexity.

No need for an interface. No need for a Range class. Don't make complexity until you're sure you need it. I strongly suspect that a single class with a 3-optional-parameter constructor will keep things much simpler.


The data I get from the server is either a DateTime object, representing the expected shipping date, or two integer values with the minimum and maximum days from today that the item will be shipped.

Use a single constructor passing all 3 values via optional parameters. This supplies all the context needed. The single constructor logic can then evaluate for all variations to set initial state correctly. Also use named parameters in the constructor call if desired to make it crystal clear.

public class ShipDate {
    public ShipDate (Datetime? shipDate = null, int earliestOffset = 0, latestOffset = 0) {
        EstShipDate = (DateTime) shipDate ?? DateTime.Now.Date;
        start = earliestOffset < 0 ? 0 : earliestOffset;
        end   = latestOffset < 0 ? 0 : latestOffset;
        // That's all, folks!
    }
}

If I standardize on dates, I'll have to convert those integers into properly constructed DateTimes,

No. Don't do this and things are simpler; instead use DateTime.AddDays()`.

public DateTime EstShipDate      {get; protected set;}
public DateTime EarliestShipDate { get { return EstShipDate.AddDays(start).Date; } }
public DateTime LatestShipDate   { get { return EstShipDate.AddDays(end).Date; } }

This is potentially more flexible if dates and offset values are allowed to change.


which would raise the complexity quite a bit due to everything that would need to be taken into account, like client vs server dates, UTC, daylight saving time differences etc. I'd like to avoid creating this kind of complexity at this point.

Read about handling time zones here.

For now just include a DateTime.DateTimeKind (enum) constructor parameter and deal with it later.


From one of the answers:

With your exact case of two dates, a date range which starts in the morning and ends in the evening seems to be the perfect solution. Or perhpas a start date and an integer number of extra days it might be?

Use the DateTime.Date property and totally ignore time.

0

What about something like

class EstimattedShippingDateDetails
{
    DateTime EarliestShippingDate {get; set;}
    DateTime LatestShippingDate {get; set;}
    TimeSpan Range 
    {
        get 
        { 
            return LatestShippingDate - EarliestShippingDate; 
        } 
    }
}

A specific date: The item is guaranteed to ship at that date

Both EarliestShippingDate and LatestShippingDate are set to the guaranteed shipping date.

A day interval: The item will be shipped "X to Y" days from today

EarliestShippingDate is set to today and LatestShippingDate is set to today + (Y - X)

0

You need to decide what is the difference (if there is any) between a single shipping date, and a range that consist of one day. If a "single shipping date" is just a range of days that happens to consist of one day, then model everything as start date and end date. If a "single shipping date" and a range of days with the same start and end date are to be treated differently then store one of the two cases, either a single date for a single shipping date, and two dates for a range of days.

0

You have basically 2 options:

  • 2 dates. If the object represents a single date, either set them both to the same value, or set the 2nd one to a null value.

  • 1 date and a timespan. The date represents the start, and the timespan show how much in the future the range can be. Set the range to 0 for a single date.

For shipping I would have a datetime rather than a date as you will no doubt want to model morning/evening delivery. Which one of the 2 options is best depends how you like to calculate the display. If you're showing "between x and y" then the first option might be easier to use, if you're showing "up to x days from y" then the latter is easier to use.

If you dislike null values then the latter option is better, as calculating the original date plus the timespan can be done regardless of whether the timespan has a value or is set to 0. You will always get a correct result without checking for null.

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.