2
\$\begingroup\$

This code is part of a data model used to create keyword records in a database.

if (_queue
    .OrderByDescending(record=>record.Primary)
    .GroupBy(record=>record.Term)
    .Select(record=>record.First())
    .Select(Create)
    .Any(result=>result <= 0))
{
    throw new ModelException("Failed to create keyword record.");
}

While I can understand this, I was also thinking it was a little obfuscated.

Is the intent of this code clearly readable?

\$\endgroup\$
0

1 Answer 1

1
\$\begingroup\$

Definitely readable although I would just combine your two select statements and add some whitespace:

if (_queue.OrderByDescending(record => record.Primary)
          .GroupBy(record => record.Term)
          .Select(record => Create(record.First()))
          .Any(result => result <= 0))
{
    throw new ModelException("Failed to create keyword record.");
}

As you see I also prefer to keep the first statement on the same line and indent the next statements to align with the linq statement.

One remark though: first you're ordering your records and then grouping them. Is this done deliberately? I would assume that you'd want to group first and then order, not in the least because you would have to order less elements. But then again: grouping might be more performant if you order it first so I'm not entirely sure.

\$\endgroup\$
2
  • \$\begingroup\$ Yeah, ordering first is done deliberately. I'm basically removing duplicate keywords, but I need to make sure that ones marked Primary (true/false) are kept marked as primary. So I just order them to be first. \$\endgroup\$
    – Reactgular
    Jan 5, 2015 at 21:11
  • \$\begingroup\$ Dropping the second Select has made this much easier to read now. Thanks. \$\endgroup\$
    – Reactgular
    Jan 5, 2015 at 21:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.