7
\$\begingroup\$

The following code fetches a set of company IDs from a database if there are any events associated with a company that has a missing property. Because the set of company IDs is embedded into the SQL query using parameters, and the number of parameters may exceed the maximum for the RDBMS / database driver, the set is partitioned into maximal subsets.

Performance aside, in terms of maintainability and ability to debug, is the following "too Linq-y"?

HashSet<int> missingCompanyIds =
    companyEvents
        .Select(ev => ev.CompanyId).ToHashSet()
        .Partition(DataContext.MaxAvailableSqlParameters(usedParameters), true)
        .SelectMany(companyIdsPartition =>
                missingEventsQuery                                           // linq-to-sql
                    .Where(ev => companyIdsPartition.Contains(ev.CompanyId)) // linq-to-sql
                    .Select(ev => ev.CompanyId)                              // linq-to-sql
                    .Distinct())                                             // linq-to-sql
        .ToHashSet();

If any of the database side-effects of the query methods cause a crash, perhaps this is harder to debug, I don't know. I am able to set breakpoints inside lambdas, but my local environment might contain fewer of the objects I wish to inspect when debugging (e.g. the missingEventsQuery)? I'm unsure if these concerns would be relevant in practice.

Would there be any advantages in the case above if some of the .Select()s were written as for loops?

\$\endgroup\$
3
\$\begingroup\$

The code looks OK. Linq can be somewhat tricky to debug, as there is some magic happening behind the scenes. In my world that still doesn't warrant to rebuild it into somehting else just for debugging for a random case which you don't experience just now.

Don't worry too much for problems which hasn't occured yet. Some say that worrying beforehand is double trouble. Firstly you worry before you've got a reason to worry, which is unneccessary. Then secondly, if you don't experience the problem, you've worried without a cause, and if it actually happens you get to worry all over again. In short, don't worry until it happens.

That does not say that you shouldn't write readable and maintable code, with good tests to test the various cases which might trigger variants of your query. I.e. you should make tests with various counts of parameters, to possibly trigger the RDBMS/database driver. A test destined to fail in a predictable manner, can still be a good test. And you should have tests displaying that the partioning is working as you expect.

\$\endgroup\$
3
\$\begingroup\$

Not so a direct answer to your question but this may help you.

Most of methods you are using are really extension methods, If your only worry is debugging or logging, you can get code for these methods and create your own extension methods with same names but Namespace must be different.

As these are extension methods, you will not be able to override them.

\$\endgroup\$
2
\$\begingroup\$

In addition to what has already been said: As you stated you use LINQ to SQL which means the LINQ query can possibly be translated straight into an SQL query executed on the server. If you translate it into a bunch of for loops then you'll loose that.

Another point I'd like to point out: You should possibly consider restructuring your query. The way I read it, it basically finds all company ids which are associated with missing events provided there are events for that company id in the first place. To my mind the simplest way to express this is the following query:

var missingCompanyIds 
          = companyEvents
                .Select(ev => ev.CompanyId)
                .Intersect(missingEventsQuery.Select(mev => mev.CompanyId))
                .ToHashSet();

This should get translated into a single query on the server, independent of the number of company ids present in the system. Whether or not it's going to be better I can't say since it will depend on your database design, the data present, etc. but I'd argue it's a lot easier to read and understand.

\$\endgroup\$
  • \$\begingroup\$ It certainly is cleaner. The reason I need the LINQ to resolve to several SQL queries is because I serialize the list of company IDs using SQL parameters, and MSSQL has an arbitrary limit of approx. 2100 parameters per query. So if the list I serialize has more than that amount of elements in it, I need to split it across several queries, painful as it is. (Ideally, these 2100+ company IDs would all be on the server side.) \$\endgroup\$ – Simon Shine Nov 24 '15 at 22:44
  • 1
    \$\begingroup\$ @SimonShine: Well, is companyEvents another query or is that a in-memory list? I guess my suggestion would simply pull all company ids belonging to missing events into memory as well in that case and perform the intersect in memory \$\endgroup\$ – ChrisWue Nov 24 '15 at 22:58

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.