diff --git a/dot-net/UnitTesting/WriteUnitTest.UnitTests/Services/LessonServiceUnitTests.cs b/dot-net/UnitTesting/WriteUnitTest.UnitTests/Services/LessonServiceUnitTests.cs index c016f412..a39f3d2b 100644 --- a/dot-net/UnitTesting/WriteUnitTest.UnitTests/Services/LessonServiceUnitTests.cs +++ b/dot-net/UnitTesting/WriteUnitTest.UnitTests/Services/LessonServiceUnitTests.cs @@ -1,4 +1,5 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; +using WriteUnitTest.Services; namespace WriteUnitTest.UnitTests.Services { @@ -6,8 +7,114 @@ namespace WriteUnitTest.UnitTests.Services public class LessonServiceUnitTests { [TestMethod] - public void UpdateLessonGrade_Test() + public void UpdateLessonGrade_ShouldPassLessonWithPassingGrade() { + // Setup + LessonService lessonService = new LessonService(); + int lessonId = 12; + double grade = 100.00d; + + // Act + lessonService.UpdateLessonGrade(lessonId, grade); + + // Assert + Assert.AreEqual(lessonService.LessonRepo.GetLesson(lessonId).IsPassed, true, "Passing grade did not pass class"); + + } + + [TestMethod] + public void UpdateLessonGrade_ShouldDoNothingIfLessonAlreadyPassed() + { + // Setup + LessonService lessonService = new LessonService(); + int lessonId = 86; + double grade = 100.00d; + + // Act + lessonService.UpdateLessonGrade(lessonId, grade); + + // Assert + Assert.AreEqual(lessonService.LessonRepo.GetLesson(lessonId).IsPassed, true, "Passing grade did not pass class"); + // This requires another check to verify that the module repository was not called, but I am not yet familiar enough with C#'s syntax for mocks + } + + [TestMethod] + public void UpdateLessonGrade_ShouldFailLessonWithFailngGrade() + { + // Setup + LessonService lessonService = new LessonService(); + int lessonId = 12; + double grade = 0.00d; + + // Act + lessonService.UpdateLessonGrade(lessonId, grade); + + // Assert + Assert.AreEqual(lessonService.LessonRepo.GetLesson(lessonId).IsPassed, false, "Failing grade did not fail class"); + + } + + [TestMethod] + public void UpdateLessonGrade_ShouldPassLessonWithGradeEqualToMinimum() + { + // Setup + LessonService lessonService = new LessonService(); + int lessonId = 12; + double grade = lessonService.ModuleRepository.GetModule(lessonId).MinimumPassingGrade; + + // Act + lessonService.UpdateLessonGrade(lessonId, grade); + + // Assert + Assert.AreEqual(lessonService.LessonRepo.GetLesson(lessonId).IsPassed, true, "Equal to passing grade did not pass class"); + + } + + [TestMethod] + public void UpdateLessonGrade_ShouldThrowExceptionWhenInvalidLessonProvided() + { + // Setup + LessonService lessonService = new LessonService(); + int lessonId = 45; + double grade = 10.0d; + + // Act + try + { + lessonService.UpdateLessonGrade(lessonId, grade); + } + catch(System.ArgumentOutOfRangeException e) + { + StringAssert.Contains(e.Message, "Lesson id not found!"); + return; + } + + // Assert + Assert.Fail("Expected exception not thrown"); + + } + + [TestMethod] + public void UpdateLessonGrade_ShouldThrowExceptionWhenLessonHasNoModule() + { + // Setup + LessonService lessonService = new LessonService(); + int lessonId = 46; + double grade = 10.0d; + + // Act + try + { + lessonService.UpdateLessonGrade(lessonId, grade); + } + catch (System.ArgumentOutOfRangeException e) + { + StringAssert.Contains(e.Message, "No module found for lesson!"); + return; + } + + // Assert + Assert.Fail("Expected exception not thrown"); } } } \ No newline at end of file diff --git a/dot-net/UnitTesting/WriteUnitTest.UnitTests/WriteUnitTest.UnitTests.csproj b/dot-net/UnitTesting/WriteUnitTest.UnitTests/WriteUnitTest.UnitTests.csproj index f3d83f84..ab1bed70 100644 --- a/dot-net/UnitTesting/WriteUnitTest.UnitTests/WriteUnitTest.UnitTests.csproj +++ b/dot-net/UnitTesting/WriteUnitTest.UnitTests/WriteUnitTest.UnitTests.csproj @@ -55,6 +55,12 @@ + + + {00A40A05-8314-4F25-A444-46DDEAC3497E} + WriteUnitTest + + diff --git a/dot-net/UnitTesting/WriteUnitTest/Program.cs b/dot-net/UnitTesting/WriteUnitTest/Program.cs index a25cc860..f473d012 100644 --- a/dot-net/UnitTesting/WriteUnitTest/Program.cs +++ b/dot-net/UnitTesting/WriteUnitTest/Program.cs @@ -6,11 +6,11 @@ public class Program { public static void Main(string[] args) { - var lessonSvc = new LessonService(); + LessonService lessonSvc = new LessonService(); - var lessonId = 12; + int lessonId = 12; - var grade = 98.2d; + double grade = 98.2d; lessonSvc.UpdateLessonGrade(lessonId, grade); } diff --git a/dot-net/UnitTesting/WriteUnitTest/Repositories/LessonRepository.cs b/dot-net/UnitTesting/WriteUnitTest/Repositories/LessonRepository.cs index e122d605..2b74a9d0 100644 --- a/dot-net/UnitTesting/WriteUnitTest/Repositories/LessonRepository.cs +++ b/dot-net/UnitTesting/WriteUnitTest/Repositories/LessonRepository.cs @@ -23,6 +23,12 @@ public LessonRepository() LessonId = 46, Grade = 0.0d, IsPassed = false + }, + new Lesson + { + LessonId = 86, + Grade = 88.2d, + IsPassed = true } }; } diff --git a/dot-net/UnitTesting/WriteUnitTest/Services/LessonService.cs b/dot-net/UnitTesting/WriteUnitTest/Services/LessonService.cs index e0f04a5b..d3378425 100644 --- a/dot-net/UnitTesting/WriteUnitTest/Services/LessonService.cs +++ b/dot-net/UnitTesting/WriteUnitTest/Services/LessonService.cs @@ -1,24 +1,60 @@ -using WriteUnitTest.Repositories; +using WriteUnitTest.Entities; +using WriteUnitTest.Repositories; namespace WriteUnitTest.Services { public class LessonService { + // Create private fields for repositories to protect them from unwanted modification + private LessonRepository _LessonRepo; + private ModuleRepository _ModuleRepository; + + public LessonService() { + /* + * Instantiate repositories in LessonService constructor so that the instance isn't instantiating + * repositories on the fly and their state at the time of LessonService's creation is preserved. + */ + LessonRepo = new LessonRepository(); + ModuleRepository = new ModuleRepository(); } + /* + * Create public getters for our private fields; protect the setters from outside modification + * as it doesn't seem necessary to allow outside classes to modify this data. + */ + public LessonRepository LessonRepo { get => _LessonRepo; protected set => _LessonRepo = value; } + public ModuleRepository ModuleRepository { get => _ModuleRepository; protected set => _ModuleRepository = value; } + + /// + /// Update the grade of a lesson and evaluate whether that lesson is passed or not. + /// Checks the provided grade against the provided lesson's module data to verify + /// that the grade is greater than the module's minimum grade. If so, changes + /// lesson's IsPassed field to true. + /// + /// The ID of the lesson to be updated + /// The grade for the lesson public void UpdateLessonGrade(int lessonId, double grade) { - var lessonRepo = new LessonRepository(); - var lesson = lessonRepo.GetLesson(lessonId); + Lesson lesson = LessonRepo.GetLesson(lessonId); + //Throw an exception if no lesson is found and allow caller to handle the error. + if(lesson ==null) + { + throw new System.ArgumentOutOfRangeException("lessonId", lessonId, "Lesson id not found!"); + } + lesson.Grade = grade; if (!lesson.IsPassed) { - var moduleRepository = new ModuleRepository(); - var module = moduleRepository.GetModule(lessonId); + Module module = ModuleRepository.GetModule(lessonId); + //Throw an exception if no module is found and allow caller to handle the error. + if (module == null) + { + throw new System.ArgumentOutOfRangeException("lessonId", lessonId, "No module found for lesson!"); + } if (grade >= module.MinimumPassingGrade) { diff --git a/dot-net/WhatWouldYouChange/INSTRUCTIONS.txt b/dot-net/WhatWouldYouChange/INSTRUCTIONS.txt index be2263d6..f1dd1bcf 100644 --- a/dot-net/WhatWouldYouChange/INSTRUCTIONS.txt +++ b/dot-net/WhatWouldYouChange/INSTRUCTIONS.txt @@ -5,4 +5,17 @@ Suggested Changes: - \ No newline at end of file + I would change the visibility of the exampleText field in ExampleClass so that the getter is public but the setter is protected or private. + As it is now, the main program can manipulate the value of exampleText, and modifying class data from outside the class is typically not good practice. + I would change the value passed by Program to be "ExampleText.txt", so that it will pass in the correct name of the text file, allowing ExampleMethod to properly read it. + I would change the catch in ExampleMethod in a few ways: + 1) Catch more specific exceptions than just a blanket catch-all; I think catching EVERY exception File.Open can throw might be overkill, but I would minimally catch + FileNotFoundException and IOException, and provide a third catch on Exception to catch other errors. + 2) The catch should respond with the details of the caught exception rather than just modifying the data with the desired value. It isn't even known that the caller of this + method _always_ wants the contents of ExampleText.txt; if a different text file is passed in with different contents (and experiences an error during open), + this method fails to perform its function. + I would use explicit types in place of var in all cases. I think this is just good practice in general. + I would add class-level and function-level comments documenting the expected behavior of the code. + I would close the streamReader when I am done with it, after reading the data. + I may change the variable name of exampleText to ExampleText, as this seems to be the standard for class fields in C#, but it would depend upon my team's style agreement. + \ No newline at end of file