HiveBrain v1.2.0
Get Started
← Back to all entries
debugcsharpModerate

Optimising and error handling Linq query

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
handlinglinqerrorqueryoptimisingand

Problem

I have 2 tables as below:

Downtime

DowntimeCode

I am then doing this to get the Description field from the DowntimeCode table for the DowntimeCode with the sum highest value in the Downtime table for the current date and a specified line:

public string TopCodeForToday(string line)
    {
        var date = DateTime.Now;

        using (var db = new PillowContext())
        {
            var qry = (from d in db.DownTimes
                        where DbFunctions.TruncateTime(d.DateTime) == DbFunctions.TruncateTime(date) && d.Line == line
                        group d by new { d.Code }
                            into g
                            let total = g.Sum(x=>x.Amount)
                            orderby total descending 
                            select new { g.Key.Code, Total = total }).ToList();

            for (int i = 0; i <= 1; i++)
            {
                foreach (var item in qry)
                {
                    int x = item.Code;
                    var result = from r in db.DownTimeCodes
                        where r.Code == x
                        select r.Description;

                    return result.FirstOrDefault();
                }                 
            }
        }
        return "Fail";
    }


Now this is working OK but I can't help but think there is a simpler way of doing it. I'm also not too hot on error handling (hence there not being any). Can anyone make any recommendations on how to improve and streamline the above?

Solution

Not having any error handling in your TopCodeForToday() method, means if anything goes wrong, you're putting the burden of catching any exceptions onto the caller.

And this isn't necessarily a bad thing. It depends on what the client code is expecting.

We don't know what class this method is a member of. The problem is that the naming of the method doesn't convey any information about what's going on, and whether the calling code should wrap it with a try/catch block. If the class is clearly an object that accesses a database and only does SELECT operations, then it's probably fine. Otherwise the method's name should say what it's doing - SelectTopCodeForToday() would be a better name regardless of all of the above, be it only because it starts with a verb.

I don't think returning a string that says "Fail" is a good idea. Replace FirstOrDefault() with First() and let it throw an exception, and then your client code will know that something is wrong with the data.

The problem with returning a "Fail" string, is that it's a valid string. And then the client code needs to verify whether the function returned "Fail" or not. By throwing, you only handle the "happy" path, making your code more focused; and you handle exceptional situations with ...exceptions.

Context

StackExchange Code Review Q#41156, answer score: 10

Revisions (0)

No revisions yet.