6
\$\begingroup\$

I have a list of patients, Patient class has Prescriptions property, which is a list of Prescription objects. Prescription has 3 important attributes, Medicament,Amount and Type(which is basically a key for joining).

I need to join a list of Prescription types, as a prescription type has an attribute Influence which servers as multiplier for the amount.

var prescriptions = (from prs in 
                        (from p in patients
                             from pr in p.Prescriptions
                             join type in DatabaseService.PrescriptionTypes
                                 on pr.Type equals type.Value
                         select new { Med = pr.Medicament,Amount = pr.Amount * (int)type.Influence })
                    group prs by prs.Med into prgs
                    select new { Medicament = prgs.Key,Amount = prgs.Sum(prg => prg.Amount) }).ToList();

Basically what I do (from inner to outer queries)

  • Join prescriptions with prescription types, get Medicament and the multiplied Amount
  • Group the records by the Medicament
  • Run one more select to get the overall sum from all patients

Is there any simpler way?

EDIT:

So I realised couple things and simplified the query quite a bit, I think it's much more readable now. The things I missed the first time were:

  • SelectMany that lets me get rid of the outter select, since prescriptions are all I care about
  • Being able to create new objects within group statement, hence one more select can be avoided

The new code looks like

    var prescriptions = from p in patients.SelectMany(p => p.Prescriptions)
                        join type in DatabaseService.PrescriptionTypes
                            on p.Type equals type.Value
                        group new { Amount = p.Amount * (int)type.Influence } by p.Medicament into prs
                        select new { Medicament = prs.Key,Amount = prs.Sum(pr => pr.Amount) };

With that I'm pretty happy and I doubt you can simplify it anymore.

\$\endgroup\$
  • \$\begingroup\$ Try using chaining rather than this version. It will make you code more readable , I dont have the entities, I could have tried to make it chainging \$\endgroup\$ – paritosh Oct 13 '15 at 9:13
1
\$\begingroup\$

You're right, I cannot simplify it anymore ;-)

Most reviews are about commenting bad code/habits and making suggestions how to improve it. But you're lucky, to me, there is nothing bad to say about your snippet so this will be a short review (and probably a reason why there are no anwers yet).

You use clear names for the properties and variables where you use the first letter of the collection name or something derived (like type). You also use meaningfull names for the properties of the anonymous objects.

If the rest of your code looks as clean as this small sample you should be happy ;-)

\$\endgroup\$

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.