8
\$\begingroup\$

We have a table with calculated data that groups sales by product, year and month, to provide fast querying for statistics.

My colleague argues that the year and month should be two separate fields, because a day is meaningless.

I want it as a date field, because using two separate fields leads to awkward code like this

var lastTwelveMonths = new DateTime(DateTime.Now.Year, DateTime.Now.Month, 1).AddMonths(-11);
var result = data.Where(item =>
    (item.Year > lastTwelveMonths.Year
    || (item.Year == lastTwelveMonths.Year && item.Month >= lastTwelveMonths.Month));

instead of this

var lastTwelveMonths = new DateTime(DateTime.Now.Year, DateTime.Now.Month, 1).AddMonths(-11);
var result = data.Where(item => item.YearAndMonthAsDate >= lastTwelveMonths);

I understand his argument, but the code is harder to read and there's a higher chance on bugs when doing date/time calculations without using date/time objects. I also can't cast it to a DateTime in the query because I'm using LINQ to Entities.

Which method is the better one?

Improve this question
\$\endgroup\$
4
  • \$\begingroup\$ Does the information come from a database or where does it come from? If it's a database, which kind of RDMS is it? \$\endgroup\$ – Simon Forsberg Jan 29 '14 at 12:27
  • \$\begingroup\$ @SimonAndréForsberg It comes from a database, SQL Server 2008 R2. \$\endgroup\$ – Stijn Jan 29 '14 at 12:28
  • \$\begingroup\$ In the chat you asked for a linq-to-entities tag, would the already existing tag entity-framework work? \$\endgroup\$ – Simon Forsberg Jan 29 '14 at 17:24
  • \$\begingroup\$ @Simon It didn't occur to me to try that tag, I've added it together with linq. Feel free to edit the tags though :) \$\endgroup\$ – Stijn Jan 29 '14 at 22:06
7
\$\begingroup\$

In general it really depends on the use cases of your data. If you need to perform queries using just year or just month then you reduce complexity in the code and queries by having separate fields.

However, if you aren't bounding by just a year or just a month then you would want to use a date field.

Professionally I use date or string fields (YYYYMMDD) when representing dates and times because the speedup you get from single year/month fields is small with todays databases (assuming things are correctly indexed).

Improve this answer
\$\endgroup\$
3
\$\begingroup\$

If you're looking for an argument by authority, consider Microsoft's aversion to GetVersionEx and in particular misuse of the OSVERSIONINFOEX structure in calculations that look suprisingly similar. By analogy to the reasons they shim the results from GetVersionEx for program compatibility, it's apparently far to easy to write code that means to say "After February 2010" as month > 2 && year > 2010, when the correct code is the more complicated year > 2010 || (year == 2010 && month > 2) as your example shows.

Per a lack of understanding of the limitations of LINQ to entities, I thought your example was needlessly complex, and exhibited a flawed argument. After all, if you can compare to a DateTime that you've just constructed, and can do the complex logic, why not just encapsulate it back into the DateTime class?

var lastTwelveMonths = new DateTime(DateTime.Now.Year, DateTime.Now.Month, 1).AddMonths(-11);
var result = data.Where(item => new DateTime(item.Year, item.Month, 1) >= lastTwelveMonths));

However as you clarified in your comments below, L2E does not support creating a DateTime from separate columns inside a Where expression. Instead it's mapping the comparison DateTime lastTwelveMonths date into database queries, and is unable to map from my counterexample to the same queries as generated in your first example.

Improve this answer
\$\endgroup\$
5
  • \$\begingroup\$ I specifically covered that in my question: I also can't cast it to a DateTime in the query because I'm using LINQ to Entities. Other than that, good answer. \$\endgroup\$ – Stijn Jan 30 '14 at 14:38
  • \$\begingroup\$ @Stijn There must be a subtlety here that I don't understand (I don't have much experience with LINQ yet), as my counterexample uses the approach of your second example with the data of the first; if my counterexample cannot be done, I don't see how your second example can work either. \$\endgroup\$ – Michael Urman Jan 31 '14 at 13:21
  • \$\begingroup\$ It's a subtle difference indeed. In my second example, item.YearAndMonthAsDate is a date field in the database, which translates to a DateTime in a L2E query. In your example, item.Year and item.Month are numeric fields in the database and you try to create a DateTime object in the query. This leads to a first problem: Only parameterless constructors and initializers are supported in LINQ to Entities. \$\endgroup\$ – Stijn Jan 31 '14 at 13:42
  • \$\begingroup\$ No worries, you might think, I'll just use the parameterless constructor and do something like this: new DateTime().AddYears(item.Year - 1).AddMonths(item.Month - 1) >= lastTwelveMonths. This doesn't work either, because it cannot be translated to an SQL query: LINQ to Entities does not recognize the method 'System.DateTime AddMonths(Int32)' method, and this method cannot be translated into a store expression. In short, some things possible in L2O are not possible in L2E. \$\endgroup\$ – Stijn Jan 31 '14 at 13:43
  • \$\begingroup\$ Thanks for the explanation! I'll update my answer accordingly. \$\endgroup\$ – Michael Urman Jan 31 '14 at 13:56

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.