-1

I have an Exam class that represents an examination/test:

public class Exam
{
    public int Id { get; set; }

    [Required]
    [StringLength(maximumLength: 30, MinimumLength = 1]
    public string Name { get; set; } 

    [Required]
    public Subject Subject { get; init; }  

    [Required]
    public Enrollment Enrollment { get; init; }

    [Required]
    public List<ExamSet> Sets { get; set; } // Sets of questions

    [Required]
    public DateTimeOffset From { get; set; } // Start Time

    [Required]
    public DateTimeOffset To { get; set; }  // End Time

    [Required, Display(Name = "Question Source Repositories")]
    public List<Repository> Repos { get; set; } 

    [Range(0, int.MaxValue)]
    public int SetCount { get; } // Number of questionnaires/question sets to be picked

    public List<int> EasyCounts { get; set; }

    public List<int> MediumCounts { get; set; }

    public List<int> HardCounts { get; set; }

    public List<int> VeryHardCounts { get; set; }

    public List<ExamSet> PickQuestions(IQuestionsPicker picker)
    {
        List<Questionnaire> questionnaires = new();
        for (int i = 0; i < SetCount; i++)
        {
            questionnaires.Add(new Questionnaire());
        }
        for (int i = 0; i < Repos.Count; i++)
        {
            Dictionary<QuestionDifficulty, int> requiredQCounts = new()
            {
                [QuestionDifficulty.Easy] = EasyCounts[i],
                [QuestionDifficulty.Medium] = MediumCounts[i],
                [QuestionDifficulty.Hard] = HardCounts[i],
                [QuestionDifficulty.VeryHard] = VeryHardCounts[i]
            };

            var pickedQuestionnaires = picker.Pick(Repos[i], SetCount, requiredQCounts);

            for (int j = 0; j < SetCount; j++)
            {
                questionnaires[j] = Questionnaire.Merge(questionnaires[j], pickedQuestionnaires[j]);
            }
        }

        List<ExamSet> sets = new();
        for (int i = 0; i < SetCount; i++)
        {
            sets.Add(new ExamSet()
            {
                Exam = this,
                Questions = questionnaires[i].AsEnumerable(),
            });
        }
        return sets;
    }
}

Properties of Exam

Type Property Description
Enrollment Enrollment All students part of this enrollment are candidates of this test
DateTimeOffset From Start Time
DateTimeOffset To End Time
List<Repository> Repos The repositories from which questions will be picked
List<int> SetCount The number of ExamSets to be picked
List<int> EasyCounts, MediumCounts, HardCounts, VeryHardCounts Number of easy, medium, hard and very hard questions to be picked for each repository. e.g.: EasyCount[0] means the number of easy questions to be picked for Repos[0].

Each Exam contains multiple ExamSets, each of which contain a different set of questions. One of these sets will be randomly assigned to someone who takes the test. Here is ExamSet:

public class ExamSet
{
    public int Id { get; set; }

    [Required, StringLength(maximumLength: 5, MinimumLength = 1)]
    public string Name { get; set; }

    [Required]
    public Exam Exam { get; set; }

    public IEnumerable<Question> Questions { get; set; }
    
}

Problem

As it can be noticed, the Exam class is quite complex. Moreover, the properties Repos, EasyCounts, MediumCounts, HardCounts, VeryHardCounts specify how the questions will be picked for this test and has nothing to do with the test. The picked question sets will be assigned to the Sets property. I wonder if using a fluent interface/ the builder pattern is appropriate here. Can someone suggest a better pattern for this?


Question picking will be handled by one of the classes that implement IQuestionPicker(strategy pattern):

public interface IQuestionsPicker
{
    List<Questionnaire> Pick(Repository repo,
                             int setCount,
                             IDictionary<QuestionDifficulty, int> questionCount);
} 

QuestionDifficulty is an enum:

enum QuestionDifficulty
{
    Easy,
    Medium,
    Hard,
    VeryHard
}

Exam Creation

Also, note that the IQuestionPicker.Pick() method takes in only one Repository but Exam has Repos which is of type List<Repository>.
I have written a method in Exam that takes in an IQuestionPicker as argument and calls Pick() for each repository and uses Questionnaire.Merge()(definition below) to combines all the picked questionnaires into a single List<Questionnaire> and converts the to List<ExamSet>. Should I include this method in the Exam class as it is right now or make a separate class for this?
If something here is unclear, please let me know.


EDIT

Exam here represents an examination event that will be held online in the future. A new Exam instance creation indicates that an examination event is scheduled to occur in the future hence the start and end times. All students part of the enrollment represented by Enrollment are the candidates of the test. (Enrollment has a property of type List<Student> ) Since, the properties Repos, EasyCounts, MediumCounts, HardCounts, VeryHardCounts are just to dictate how the questions will be picked and from which repositories, I am not sure if I need to store these option properties to the database in the first place.


Sorry for the confusion that resulted, I've included the definition of Questionnaire. It is just a container for a dictionary(or hash map) that stores a set of questions for each difficulty. It has an AsEnumerable() method that just combines these sets and returns them a single collection. Also a bunch of Add() methods that adds a question to the right set based on its difficulty.

public class Questionnaire 
{
    public Dictionary<QuestionDifficulty, HashSet<Question>> Questions { get; } = new();

    public IEnumerable<Question> AsEnumerable() => Questions.SelectMany(kvp => kvp.Value);
    
    //Indexer (allows using [] operator with a Questionnaire object) 
    public HashSet<Question> this[QuestionDifficulty d] => Questions[d];

    public void Add(Question question)
    {
        var difficulty = question.Difficulty;
        if (!Enum.IsDefined(difficulty))
        {
            throw new InvalidOperationException($"Question has invalid difficulty {difficulty}");
        }
        if(!Questions.ContainsKey(difficulty))
        {
            Questions[difficulty] = new();
        }
        Questions[difficulty].Add(question);
    }
    public bool TryAdd(Question question)
    {
        try { Add(question); return true; }
        catch(InvalidOperationException) { return false; }
    }

    public void AddRange(IEnumerable<Question> questions)
    {
        foreach(var question in questions) { Add(question); }
    }

    public bool TryAddRange(IEnumerable<Question> questions)
    {
        try { AddRange(questions); return true; }
        
        catch(InvalidOperationException) { return false; }
    }

    public bool Contains(Question question) => Questions
        .Values
        .Any(qset => qset.Contains(question));

    public static Questionnaire Merge(params Questionnaire[] questionnaires)
    {
        Questionnaire questionnaire = new();
        foreach(var q in questionnaires)
        {
            questionnaire.AddRange(q.AsEnumerable());
        }
        return questionnaire;
    }
}

This is one of the concrete implementations of IQuestionPicker that picks unique questions for each set(Questionnaire):

public class UniqueQuestionsPicker : IQuestionsPicker
{
    public List<Questionnaire> Pick(Repository repo, int setCount, IDictionary<QuestionDifficulty, int> questionCountsPerSet)
    {
        var questions = repo.Questions;
        var groups = questions.GroupBy(q => q.Difficulty);
        var random = new Random();
        List<Questionnaire> questionnaires = new();

        // Add as many new empty Questionnaires as the required set count
        for(int i = 0; i < setCount; i++) { questionnaires.Add(new()); }

        foreach (var grp in groups)
        {
            //Validate if there are enough questions:
            var difficulty = grp.Key;
            var requiredQuestions = questionCountsPerSet[difficulty] * setCount; 
            var actualQuestions = grp.Count();

            if(requiredQuestions > actualQuestions)
            {
                var msg = $"Not enough questions of difficulty \"{difficulty}\" in repository ";
                throw new InvalidOperationException("Not enough questions in repository")
                {
                    Data = 
                    { 
                        ["ActualQuestions"] = actualQuestions,
                        ["RequiredQuestions"] = requiredQuestions 
                    }
                };
            }

            var shuffledQuestions = grp.OrderBy(q => random.Next());
            
            for(int i = 0; i < setCount; i++)
            {
                questionnaires[i]
                    .AddRange(shuffledQuestions
                        .Skip(questionCountsPerSet[difficulty] * i) 
                        .Take(questionCountsPerSet[difficulty]));
            }
        }
        return questionnaires;
    }
}

I have also updated the Exam class to include the conversion method that converts from List<Questionnaire> to List<ExamSet>. I can refactor any of the above classes/methods. Creating an Exam includes the following:

  • Specifying the audience (enrollment).
  • Configuring question picking.
  • Assigning scores to questions.
  • Set Assignment to each student in the enrollment.
  • Setting the start and end time(scheduling).

This is why I thought of the builder pattern/fluent interface or making a class for each of these procedures. It'd be great if someone could recommend the best practice.

10
  • 1
    I am not clear if your Exam class represents a test instrument (which contains questions, and eventually receives a grade) or it refers to an examination event (which would have a start time, an end time, possibly a room number, and a test instrument, which in turn contains the questions). Your model might be missing an important entity in this regard. As for who should call Pick(), that should probably be a private method inside of an ExamFactory of some kind.
    – John Wu
    Apr 5, 2021 at 5:32
  • 1
    @AmalK John's point wasn't specifically about room numbers, but rather on the difference between "exam" being a specific test taken by a specific person, or being a test template (or preset). Consider the difference between a word document (which is a template) and having printed this document 5 times (= 5 specific instances of that template). Is your Exam class more closely related to that Word document, or to a particular printout of that Word document?
    – Flater
    Apr 7, 2021 at 12:07
  • 1
    @AmalK: Parts of your question contradict that idea. (1 - comment) "The Exam class here represents a test instance" (2 - question) "Each Exam contains multiple ExamSets, each of which contain a different set of questions. One of these sets will be randomly assigned to someone who takes the test." Therefore, an Exam wouldn't be representing a specific test, if it contains multiple possible sets of questions, when a test itself would contain one of those sets.
    – Flater
    Apr 7, 2021 at 14:26
  • 1
    @AmalK: Word definitions matter when discussing concepts by using those definitions. I suspect you are thinking of an exam as a scheduled batch of tests being taken, i.e. "Class 5 will be taking the math 101 exam next Monday", but there are different interpretations of "exam", e.g. (1) "John failed his math 101 exam" (= one specific set of questions), or (2) "The teacher wrote the math 101 exam" (= just a bunch of questions, not on a specific date or for specific people yet). Your code seems to be very confused between the possible stages of an "exam" (writing, planning, taking, grading)
    – Flater
    Apr 7, 2021 at 14:32
  • 1
    Just to clarify, based on your own list of what an exam entails: (writing) Configuring question picking (planning) Specifying the audience, setting the start and end time (taking) Set assignment to each student in the enrollment (grading) Assigning scores to questions. These things all happen at very different stages, each of which need to be persisted separately and have their own meaning/life cycle, but you've conflated them into a single Exam amalgam, making it impossible to figure out what precisely Exam is meant to contribute to the application at large.
    – Flater
    Apr 7, 2021 at 14:38

3 Answers 3

1
+100

Recap from comment thread

Just recapping what was established in the comment discussion, as it explains the decisions I will make in the answer.

My comment

Word definitions matter when discussing concepts by using those definitions. I suspect you are thinking of an exam as a scheduled batch of tests being taken, i.e. "Class 5 will be taking the math 101 exam next Monday", but there are different interpretations of "exam", e.g.

  1. John failed his math 101 exam (= one specific set of questions)
  2. The teacher wrote the math 101 exam (= just a bunch of questions, not on a specific date or for specific people yet).

Your code seems to be very confused between the possible stages of an "exam" (writing, planning, taking, grading). Based on your own list of what an exam entails:

  • (writing) Configuring question picking
  • (planning) Specifying the audience, setting the start and end time
  • (taking) Set assignment to each student in the enrollment
  • (grading) Assigning scores to questions.

These things all happen at very different stages, each of which need to be persisted separately and have their own meaning/life cycle, but you've conflated them into a single Exam amalgam, making it impossible to figure out what precisely Exam is meant to contribute to the application at large.

Your reply

It completely makes sense. I've ended up with a large monolithic Exam class violating the single responsibility principle. Is it advisable to create separate classes and dealing with each of these phases independently? I was under the assumption that since all these are "parts" of an examination, Exam is a complex object and I need to construct it step by step using the builder pattern.

And yes, I've been thinking about Exam here as a collection of multiple question sets, each set containing the same number of questions for each difficulty. It wouldn't matter what set the student is assigned, it is equivalent of taking that one Exam. Each student is only assigned one of those sets. It seems like I need to add a List<Student> property to ExamSet indicating which students have been assigned that set.


Step 1 - Dictionary

Part of the issue here is the ambiguity in what the word "exam" refers to, from a conceptual point of view. What really helps here is defining specific words and linking them to concepts. Most non-trivial application analysis will contain such a dictionary.

Here's my suggested nomenclature. Change it if you want, but this helps explain later parts of this answer.

Concept Description
Exam A test template, which denotes the subject matter. For example, Math 101.
Set A collection of questions, acting as a single "variant" of the related exam. All sets of a given exam are assumed to be equal in terms of grading. Exams can have multiple sets, but passing one set equates to passing the related exam.
Question Part of the exam template, these are the questions written by the teacher. For sake of the example, I'm going to assume simple "textbox" style questions where you expect the answer to match a predetermined string value.
Schedule/Planning A planned date on which the exam takes place. For sake of the example, I assume that students are planned individually, rather than e.g. per class group.
Test When a student takes an exam, a test keeps track of their performance on that particular exam.
Test question Keeps track of the student's answers on the asked exam question.
Teacher Those who create the exam and write the questions. Note that I'm using a generalized Person class here for the sake of the example, and I'm also assuming that these teachers are the ones planning the examinations.
Student Those who take the exam. Note that I'm using a generalized Person class here for the sake of the example.

I've written the entity classes assuming EF Code First conventions, but I've omitted the navigational properties for readability's sake.


Step 2 - Writing

The exam writing phase is one where the teacher creates and configures the exam and adds questions to it, and generates as many exam sets as they want.

For the sake of user experience, I strongly suggest that these entities can be created/altered individually, instead of having to do it all in a bulk transaction, but this is your implementation choice.

The Exam tracks the exam (template):

public class Exam
{
    public int Id { get; set; }
    public string Subject { get; set; }
}

How many question of each difficulty should be put into an exam set, should be configured on the exam. However, instead of hardcoding a column per difficulty level, it is better to abstract this into a child table, so that you don't need to rewrite your columns if you add/remove difficulty levels.

public class ExamQuestionCount
{
    public int Id { get; set; }
    public QuestionDifficulty Difficulty { get; set; }
    public int Amount { get; set; }
    
    public int ExamId { get; set; }
}

So, for example, if my exam has 1 hard, 1 medium and 3 easy questions, you would have 3 ExamQuestionCount rows, one for each of the mentioned difficulty levels.

Questions are tracked as a separate collection, related to the exam (not the exam set yet!). These act as the question "repository" to generate exam sets from.

public class Question
{
    public int Id { get; set; }
    public QuestionDifficulty Difficulty { get; set; }
    public string Description { get; set; } // the question text
    public string ExpectedAnswer { get; set; }
    
    public int ExamId { get; set; }
}

Exam sets are a direct child of the exam:

public class ExamSet
{
    public int Id { get; set; }
    public string Name { get; set; }
    
    public int ExamId { get; set; }
}

And Question entities can be related to a given exam set:

public class ExamSetQuestion
{
    public int Id { get; set; }
    
    public int ExamSetId { get; set; }
    public int QuestionId { get; set; }
}

Note that it's perfectly possible for the same question to appear in more than one exam set.

As a basic idea on how to generate the exam set questions randomly:

  • Create a new ExamSet
  • Look up the ExamQuestionCount entities for the given exam.
  • For each ExamQuestionCount entity, fetch all the Question entities of the exam with that specific difficulty level.
  • Take X random questions (where X is ExamQuestionCount.Amount)
  • For each selected question, create a new ExamSetQuestion which relates to the current ExamSet and selected Question.

Step 3 - Planning

Planning an exam means setting a date. However, that should not be tracked as part of the Exam itself, since you might schedule the same exam multiple times (e.g. every year, or for different class groups, or a second one for valid absentees, or ...)

public class ScheduledExam
{
    public int Id { get; set; }
    
    public DateTime Date { get; set; }
    public int ExamId { get; set; }
}

Students are then individually invited to the scheduled exam (note: you might want to invite a class instead of individual students, but I tried keeping it simple for the answer).

public class Registration
{
    public int Id { get; set; }
    
    public int ScheduledExamId { get; set; }
    public int StudentId { get; set; }
    public bool WasPresentForExam { get; set; }
}

Step 4 - Taking the exam

I'm skipping over the specific of how the exam is taken, I'm focusing on what you track.

When the student logs in, their Test is created and starts. A random exam set is assigned.

public class Test
{
    public int Id { get; set; }
    
    public int PlannedExamId { get; set; }
    public int ExamSetId { get; set; }
    public int StudentId { get; set; }
    public bool IsFinished { get; set; }
}

This will be the main entity that tracks the student's overall performance on the exam. For simplicity's sake, I've omitted things like score tracking, as this can be dynamically calculated based on the TestQuestion entities. For performance, you might want to store a summarized count of the students score on the Test entity itself.

We then track each answer given to each question:

public class TestQuestion
{
    public int Id { get; set; }
    
    public int TestId { get; set; }
    public int QuestionId { get; set; }
    
    public string GivenAnswer { get; set; }
    public bool IsCorrect { get; set; }
}

Notice the IsCorrect flag, instead of dynamically looking for equality between Question.ExpectedAnswer and TestQuestion.GivenAnswer. While you'll initially set the boolean based on that equality, having the boolean property means that a teacher can override the boolean, e.g. if they want to give a student the marks for getting a correct answer that was wrongly flagged as false due to a typo.


Step 5 - Grading

We already tracked everything we need. We can calculate a score based on the TestQuestion entities.

int totalQuestions = myTest.TestQuestions.Count();
int correctAnswers = myTest.TestQuestions.Count(q => q.IsCorrect);

int score = 100 * (double)correctAnswers / totalQuestions;

You could, for example, calculate this score at the end of the test and store it in Test. This saves you from having to recalculate it every single time. However, beware manual overrides to TestQuestion.IsCorrect. When a teacher changes this, this means that the test score needs to be recalculated.


Business logic

I assume you'll have noticed that I didn't add any logic in here. That is because data entities should not contain logic, as they merely act as the DTO between the database and your runtime.
I know a few StackExchange users who disagree with that, but I stand by that point. Persistence entities should not contain logic.

The actual logic, such as the question picking logic and all logic involving making the decision to create one of these entities, should exist in a separate layer, i.e. the business layer. It is that business layer which will call the persistence layer methods when it so chooses.

This kind of architecture is too complex to explain from scratch in this answer, I suggest reading up on things like Clean Architecture to get an understanding of how these layers should and shouldn't interact with one another. There are many possible approaches here and opinions vary on some of the smaller implementation details.


Conclusion

The above example is heavily oversimplified, but it gives you an overview on how to separate your god entity into separate entities which each have their own specific life cycle which relates to certain features that your application needs.

4
  • Thanks a lot! I've noticed you've used an additional entity ExamSetQuestion instead of adding an IEnumerable<Question> in the ExamSet entity. Are there advantages of doing this?
    – Amal K
    Apr 10, 2021 at 5:48
  • The business logic part could be a whole question but it's opinion-based. My application is an ASP.NET Core MVC app with all these entities in the Models folder. What folder would you usually add the business logic in? How will the classes be named? I've come across many examples with the entire business logic in the controller, but it doesn't feel right.
    – Amal K
    Apr 10, 2021 at 5:54
  • 1
    @AmalK "instead of adding an IEnumerable<Question> in the ExamSet entity" I specifically omitted navigation properties for brevity's sake. You'd have a similar list navigational property, but you need the cross table to make the many-to-many relationship work (that being said, EF generally does support implicit many-to-manies, but I didn't want to overcomplicate it.
    – Flater
    Apr 10, 2021 at 22:32
  • 1
    @AmalK "What folder would you usually add the business logic in?" It's not about what folder, it's about having separate projects and have them depends on one another. This is a large chapter and I strongly suggest looking up online tutorials or blogs to get a grasp on this.
    – Flater
    Apr 10, 2021 at 22:33
1

It sounds like the conversion from List<Questionnaire> to List<ExamSet> is extremely sensitive. In some situations, its logic could even be evidence in a legal dispute, e.g. if someone suffered financial loss due to the exam and wanted to subpoena the way the questions were chosen. It is its own "concern" (as in separation of concerns) and deserves its own class so that it can version independently from everything else.

The requirement for this class is that it create something, so a safe choice is to call it a factory.

class ExamSetFactory : IExamSetFactory
{
    protected readonly IQuestionPicker _picker; //injected

    public ExamSetFactory(IQuestionPicker picker)
    {
        _picker = picker;
    }

    public List<ExamSet> CreateExamSets(List<Questionnaire> questions) //Plus any other arguments you need to pass
    {
        //Conversion logic goes here
    }
}

If you are not using dependency injection, another fun way to isolate it is to just make it an extension method.

static class ExamSetExtensions
{
    static public IEnumerable<ExamSet> ToExamSet(this List<Questionnaire> questions)
    {
        //Conversion logic goes here
    }
}

In this case the preferred return type is IEnumerable<ExamSet> in order to give the caller control over when the list is realized, e.g. if they are using LINQ and your method is only one call of many.

1
  • This clears up the query about where I should put my Pick() method, thanks a lot for your help. I have included the conversion method in the Exam class that converts from List<Questionnaire> to List<ExamSet> for clarity and also the definition of Questionnaire. Now I'm looking for a way to break my exam class down as you said as a separate class for the test instrument and examination event and the options for picking questions. Thanks again for your help :)
    – Amal K
    Apr 5, 2021 at 10:02
1

As it can be noticed, the Exam class is quite complex. I wonder if using a fluent interface/ the builder pattern is appropriate here.

I don't think the Exam class is very complex. So far Exam only has getter and setters, which might be indicative of a Anemic domain model (https://en.wikipedia.org/wiki/Anemic_domain_model) In stead of starting with the class having the property Enrollment, you can start with methods your class should have:

// enrolls a student to the exam
public void enroll(Student) { ... }

public ExamSet getSetForStudent(Student) { }

This might very well remove the need for having a public getter for Enrollment and ExamSet, since now instead of writing:

exam.Set.get(student);

You can write:

exam.getSetForStudent(student);

This is following the Law of Demeter and helps hiding away internal details.

Moreover, the properties Repos, EasyCounts, MediumCounts, HardCounts, VeryHardCounts specify how the questions will be picked and has nothing to do with the test. Can someone suggest a better pattern for this? Question picking will be handled by one of the classes that implement IQuestionPicker(strategy pattern):

Since question picking will be implemented by a strategy object, you can move the configuration to that strategy object.

public Interface IQuestionPicker 
{
   public List<Questionnaire> Pick();
}
public class DifficultyLevelQuestionPicker { 
    public List<Questionnaire> Pick();

    [Required, Display(Name = "Question Source Repositories")]
    public List<Repository> Repos { get; set; } 

    [Range(0, int.MaxValue)]
    public int SetCount { get; } // Number of questionnaires/question sets to be picked

    public List<int> EasyCounts { get; set; }

    public List<int> MediumCounts { get; set; }

    public List<int> HardCounts { get; set; }

    public List<int> VeryHardCounts { get; set; }
};

Notice that Pick doesn't need any parameters anymore since all the configuration needed is part of the concrete QuestionPicker implementation. This allows you to have simpler implementations that don't need the difficulty counts, or more complex ones that have more levels of difficulty.

I need to write a method that takes in an IQuestionPicker as argument and calls Pick() and converts the List to List. Should I include this method in the Exam class or make a separate class for this? If something here is unclear, please let me know.

I would just create a Questionnaire on the fly, every time a student enrolls. Btw. your Picker returns List<Questionnaire> but you ExamSet contains IEnumerable<Question> is there a difference between the too, or could that just be the same?

1
  • I really like the idea of moving the configuration to the strategy object. Sorry for the confusion regarding Questionnaire and ExamSet. I have included the definition of Questionnaire and also the conversion to ExamSet in the Exam class. Basically, the examiner creates an exam - specifies the configuration and size of the sets. And once created, every student enrolled will have this test scheduled for them and they will be assigned one of the sets.
    – Amal K
    Apr 5, 2021 at 10:07

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.