2
\$\begingroup\$

I have this piece of code in my service layer and registers and student for a course. As you can see if have alot of if statements and i was wondering if any could suggest a nicer way of writing this code?

    public async Task<bool> RegisterStudentForCourseAsync(RegisterStudentForCourseRequest registerStudent)
    {
        var student = await _studentRepository.GetStudentInformationAsync(registerStudent.StudentId);

        if (student == null)
        {
            throw new StudentCourseRegistrationException("Student Not Found");
        }
       
        var alreadyRegistered = student.Enrollments.Any(x => x.CourseId == registerStudent.CourseId);

        if (alreadyRegistered)
        {
            throw new StudentCourseRegistrationException("Student is Already Registered for Course");
        }
       
        //change this to a config setting
        if (student.SubjectCount >= 5)
        {
            throw new StudentCourseRegistrationException("Cannot Register Amount of Courses");
        }

        var course = _courseRepository.GetCourseInformation(registerStudent.CourseId);

        if(course == null)
        {
            throw new StudentCourseRegistrationException("Course Not Found");
        }
        
        if(course.Enrolled >= course.Capacity)
        {
            throw new StudentCourseRegistrationException("Course is fully Booked");
        }

        return await _courseRepository.SaveRegisterationOfStudentForCourse(course, registerStudent, student);
    }
}
\$\endgroup\$
  • \$\begingroup\$ Your title should stay what your code does. codereview.stackexchange.com/help/how-to-ask \$\endgroup\$ – BCdotWEB Jul 12 at 22:02
  • \$\begingroup\$ Please show more of your code, not just this method. \$\endgroup\$ – Reinderien Jul 12 at 22:04
  • \$\begingroup\$ The only other thing is the StudentCourseRegistrationException. it inherits the exception class \$\endgroup\$ – Great Khan 2016 Jul 12 at 22:07
4
\$\begingroup\$

you can write fluent extension methods:

 public static class Extensions {
     public static T EnsureNotNull<T>(this T t, string message = null) {
         if (t == null) throw new StudentCourseValidationException(message ?? "Value not found");
         return t;
     }
     public static Student EnsureAlreadyRegistered(this Student student, string courseId) {
          if (student.Enrollments.Any(x => x.CourseId == courseId) throw new StudentCourseRegistrationException("Student is Already Registered for Course");
          return student;
      }
      //and so on
 }

then your code would look like:

public async Task<bool> RegisterStudentForCourseAsync(RegisterStudentForCourseRequest registerStudent)
    {
        var student = await _studentRepository.GetStudentInformationAsync(registerStudent.StudentId);

        student
              .EnsureNotNull()
              .EnsureNotAlreadyRegistered(registerStudent.CourdeId)
              .EnsureCanHaveMoreCourses();
        
        var course = _courseRepository.GetCourseInformation(registerStudent.CourseId);

        course
             .EnsureNotNull()
             .EnsureNotFullyBooked();

        return await _courseRepository.SaveRegisterationOfStudentForCourse(course, registerStudent, student);
    }
}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ What Nudget package is it? I am facing some issues trying to reproduce the code \$\endgroup\$ – Great Khan 2016 Jul 12 at 22:05
  • \$\begingroup\$ Also is it possible to unit test this \$\endgroup\$ – Great Khan 2016 Jul 12 at 22:11
  • \$\begingroup\$ There is no nuget package, you need to write your "Extensions" class. To Unit Test it you need to mock the repositories (as you should have done before). Can you tell which problems are you facing? \$\endgroup\$ – ddfra Jul 12 at 22:12
  • \$\begingroup\$ Edit: I forgot to add the return statements in the extension methods. \$\endgroup\$ – ddfra Jul 13 at 8:03
  • \$\begingroup\$ Does the static class Extension need to have Extensions <T>. I get an error EnsureNotNull(T, string)': not all code paths return a value Service. \$\endgroup\$ – Great Khan 2016 Jul 13 at 8:03

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.