0
\$\begingroup\$

I have a PupilService which is calling the PupilRepository.AttachPupil() method.

I have an N to M relation between SchoolclassCode and Pupil.

Technical logic:

A pupil can be related to many SchoolclassCode like "Math 7 a" or "English 8 c".

Business logic:

  1. A pupil can be related only to SchoolclassCode like "Math 7 a" or "English 7 c". That means it's not possible for a pupil to belong to 2 different class numbers.

  2. A pupil can only be attached to a smaller class number during the year (changes happen in mid-year) thus all higher class relations must be removed.


The business logic from 1) and 2) should be put elsewhere. It just does not fit into the PupilService or another Repository method because by splitting up the tasks in finer grained method It would causes more queries to do the same job. Further more if I split up the business logic the repo.AttachPupil would only work the half if the repository would be called directly. I do not like that either.

public async Task AttachPupil(int pupilId, int targetSchoolclassCodeId)
{
    var pupil = new Pupil { Id = pupilId };
    context.Pupils.Attach(pupil);
    var targetSchoolclassCode = await context.SchoolclassCodes.SingleAsync(s => s.Id == targetSchoolclassCodeId);
    await context.Entry(pupil).Collection(p => p.SchoolclassCodes).LoadAsync();
    var existingLowerSchoolclasses = pupil.SchoolclassCodes.Where( s => s.SchoolclassNumber < targetSchoolclassCode.SchoolclassNumber);
    if (existingLowerSchoolclasses.Any())
    {
        throw new PupilAdvancedNextSchoolclass("A pupil can only downgrade a class within a schoolyear");
    }
    else
    {
        foreach (var schoolclassCodeToDetach in existingLowerSchoolclasses)
        {
            // Detach pupil from higher SchoolclassCode classes e.g. class 9
            pupil.SchoolclassCodes.Remove(schoolclassCodeToDetach);
        }

        // Attach Pupil to a lower class becaue his grades/marks are too bad e.g. in the mid-year to class 8
        targetSchoolclassCode.Pupils.Add(pupil);
        await context.SaveChangesAsync();
    }
}

How would you refactor this AttachPupil method so that the business logic is better separated from the data access logic?

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

First of all I'd say that your foreach loop will never loop, because it is only reached when there are no elements in existingLowerSchoolclasses.

In general I would recommend to re-think your datastructure, especially with a view towards next year and the year after. Because then you'll wish you had identified classes like "English 8 c 2014" or "Math 7 d 2015".

This will make it easier to enforce the constraints you described.

If you're concerned about the number of queries you could use a stored transaction. Even that feels old-fashioned, in some cases it still makes sense to guard the access to N:M relations with those things. The question is always: Is it really a domain problem, or is it caused by the fact that your data structure doesn't adhere enough to the domain? If you can answer that, you'll know where to put the Data Access Logic.

Last of all let me be honest: Whenever I see a N:M relation I get very suspicious about it. In most real-world situations, there are no anonymous N:M relations. A student started a class at a certain time and left it at another, also different students could have different mentors within one course, and there might be much more attributes you might want to assign to that relationship in the future. Therefore I would advise you to remodel this relation, e.g. by introducing a ClassMembership entity.

\$\endgroup\$
  • \$\begingroup\$ What you miss maybe is that there are more entities which I did not mention. Because of the YEAR in the schoolclasscode... I have 1 Schoolyear has N SchoolclassCode relation :-) \$\endgroup\$ – Elisa Jun 30 '15 at 20:06
  • \$\begingroup\$ and that is the problem I see "... or is it caused by the fact that your data structure doesn't adhere enough to the domain? " \$\endgroup\$ – Elisa Jun 30 '15 at 20:08
  • \$\begingroup\$ Yes you are right that this is actually a Schoolclass 1 to N Pupil relation and a Pupil N to M Subject relation. With the combination of Schoolclass + Subject entity things can get shorter/easier but now I see also introduce more side effects... caused by exactly the data structure. If I would have Schoolclass + Subject I would not have this kind of pseudo business logic! \$\endgroup\$ – Elisa Jun 30 '15 at 20:17

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.