From e7550d46673dba2acf64ef1a9fa29d3c27259ade Mon Sep 17 00:00:00 2001 From: Dave Teske Date: Tue, 26 Mar 2019 20:35:23 -0400 Subject: [PATCH 1/2] INSTRUCTIONS.txt updated with suggested changes. --- dot-net/WhatWouldYouChange/INSTRUCTIONS.txt | 30 ++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/dot-net/WhatWouldYouChange/INSTRUCTIONS.txt b/dot-net/WhatWouldYouChange/INSTRUCTIONS.txt index be2263d6..c8225e49 100644 --- a/dot-net/WhatWouldYouChange/INSTRUCTIONS.txt +++ b/dot-net/WhatWouldYouChange/INSTRUCTIONS.txt @@ -5,4 +5,32 @@ Suggested Changes: - \ No newline at end of file + +Snippet +- Use more descriptive names, though in this case it makes some sense not to go overboard. Specifically ExampleMethod() could have been ReadExampleFile(). + +- Property should start with a capital letter. (ExampleText) + +- Catch specific exceptions in this case at least: catch (FileNotFoundException e) + +- Use using statement to handle the disposal of the File and StreamReader + +- Don't concantenate the text in the catch block. use the "@" string prefix ex: +text = @"Lorem ipsum dolor +facilisis auctor +euismod nisi" + +- Depending on the use case it may be worth converting the fail state (Lorem ipsum...) text into a constant and moving it into a seperate file. + +- Add guard clauses for the filename parameter. + - empty + - not null + - does the file exist (though only needed if we need to do something specific on that error condition ie logging ) + +- Add logging, especially in the catch block. + +- Don't start the catch block with text = text + "...". There is the possiblity that the text variable may contain part of the stream. I think it's unlikely but always better to be safe. + +- In this case the text varaible is unnecessary assignments could be made directly to the property exampleText. + +- Unless there is a reason not to, make the setter private. \ No newline at end of file From 3e07eb61a77c331cfcea232c78c1868c79b724a2 Mon Sep 17 00:00:00 2001 From: Dave Teske Date: Tue, 26 Mar 2019 21:33:54 -0400 Subject: [PATCH 2/2] Lesson and Module repos refactored to add interfaces. LessonService refactored to add DI. Unit tests added. --- .gitignore | 8 +- .../Services/LessonServiceUnitTests.cs | 114 +++++++++++++++++- .../WriteUnitTest.UnitTests.csproj | 6 + .../Repositories/ILessonRepository.cs | 9 ++ .../Repositories/IModuleRepository.cs | 9 ++ .../Repositories/LessonRepository.cs | 2 +- .../Repositories/ModuleRepository.cs | 2 +- .../WriteUnitTest/Services/LessonService.cs | 18 ++- .../WriteUnitTest/WriteUnitTest.csproj | 2 + 9 files changed, 162 insertions(+), 8 deletions(-) create mode 100644 dot-net/UnitTesting/WriteUnitTest/Repositories/ILessonRepository.cs create mode 100644 dot-net/UnitTesting/WriteUnitTest/Repositories/IModuleRepository.cs diff --git a/.gitignore b/.gitignore index 6c00c877..6c06a5ff 100644 --- a/.gitignore +++ b/.gitignore @@ -6,4 +6,10 @@ build/ # Package Files # *.jar -*.log \ No newline at end of file +*.log +/dot-net/UnitTesting/.vs/WriteUnitTest/v15 +/dot-net/UnitTesting/WriteUnitTest/bin/Debug +/dot-net/UnitTesting/WriteUnitTest/obj/Debug +/dot-net/UnitTesting/WriteUnitTest.UnitTests/bin/Debug +/dot-net/UnitTesting/WriteUnitTest.UnitTests/obj/Debug +/dot-net/WhatWouldYouChange diff --git a/dot-net/UnitTesting/WriteUnitTest.UnitTests/Services/LessonServiceUnitTests.cs b/dot-net/UnitTesting/WriteUnitTest.UnitTests/Services/LessonServiceUnitTests.cs index c016f412..2ccffb2b 100644 --- a/dot-net/UnitTesting/WriteUnitTest.UnitTests/Services/LessonServiceUnitTests.cs +++ b/dot-net/UnitTesting/WriteUnitTest.UnitTests/Services/LessonServiceUnitTests.cs @@ -1,4 +1,9 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; +using System.Collections.Generic; +using System.Linq; +using WriteUnitTest.Entities; +using WriteUnitTest.Repositories; +using WriteUnitTest.Services; namespace WriteUnitTest.UnitTests.Services { @@ -6,8 +11,115 @@ namespace WriteUnitTest.UnitTests.Services public class LessonServiceUnitTests { [TestMethod] - public void UpdateLessonGrade_Test() + public void UpdateLessonGrade_PassingGradeSetWithAGradeGreaterThanMinimumPassingGrade() { + var fakeLessonRepo = new LessonRepoFake(); + var fakeModuleRepo = new ModuleRepoFake(); + var lessonId = 1; + var lessonService = new LessonService(fakeLessonRepo, fakeModuleRepo); + + lessonService.UpdateLessonGrade(lessonId, 81); + + Assert.IsTrue(fakeLessonRepo.lessonList.FirstOrDefault(x => x.LessonId == lessonId).IsPassed); + } + + [TestMethod] + public void UpdateLessonGrade_PassingGradeSetWithAGradeEqualToMinimumPassingGrade() + { + var fakeLessonRepo = new LessonRepoFake(); + var fakeModuleRepo = new ModuleRepoFake(); + var lessonId = 1; + var lessonService = new LessonService(fakeLessonRepo, fakeModuleRepo); + + lessonService.UpdateLessonGrade(lessonId, 80); + + Assert.IsTrue(fakeLessonRepo.lessonList.FirstOrDefault(x => x.LessonId == lessonId).IsPassed); + } + + [TestMethod] + public void UpdateLessonGrade_FailingGradeSetWithAGradeLessThanMinimumPassingGrade() + { + var fakeLessonRepo = new LessonRepoFake(); + var fakeModuleRepo = new ModuleRepoFake(); + var lessonId = 1; + var lessonService = new LessonService(fakeLessonRepo, fakeModuleRepo); + + lessonService.UpdateLessonGrade(lessonId, 79); + + Assert.IsFalse(fakeLessonRepo.lessonList.FirstOrDefault(x => x.LessonId == lessonId).IsPassed); + } + + // Additional tests should include + // Invalid LessonIds + // Grades below 0 or above 100 (assuming this matches the business rules) + // Module with no MinimumPassingGrade + + } + + + public class LessonRepoFake : ILessonRepository + { + public readonly List lessonList; + + public LessonRepoFake() + { + lessonList = new List + { + new Lesson + { + LessonId = 1, + Grade = 63.7d, + IsPassed = false + }, + new Lesson + { + LessonId = 9, + Grade = 0.0d, + IsPassed = false + } + }; + } + + public Lesson GetLesson(int lessonId) + { + return lessonList.FirstOrDefault(x => x.LessonId == lessonId); + } + } + + public class ModuleRepoFake : IModuleRepository + { + public readonly List moduleList; + + public ModuleRepoFake() + { + moduleList = new List + { + new Module + { + ModuleId = 873, + MinimumPassingGrade = 80, + Lessons = new List + { + new Lesson + { + LessonId = 1, + Grade = 63.7d, + IsPassed = false + }, + new Lesson + { + LessonId = 9, + Grade = 88.2d, + IsPassed = true + } + } + } + }; + } + + public Module GetModule(int lessonId) + { + return moduleList.FirstOrDefault(x => x.Lessons.Any(y => y.LessonId == lessonId)); } } } \ 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/Repositories/ILessonRepository.cs b/dot-net/UnitTesting/WriteUnitTest/Repositories/ILessonRepository.cs new file mode 100644 index 00000000..3f9f3959 --- /dev/null +++ b/dot-net/UnitTesting/WriteUnitTest/Repositories/ILessonRepository.cs @@ -0,0 +1,9 @@ +using WriteUnitTest.Entities; + +namespace WriteUnitTest.Repositories +{ + public interface ILessonRepository + { + Lesson GetLesson(int lessonId); + } +} \ No newline at end of file diff --git a/dot-net/UnitTesting/WriteUnitTest/Repositories/IModuleRepository.cs b/dot-net/UnitTesting/WriteUnitTest/Repositories/IModuleRepository.cs new file mode 100644 index 00000000..6a885636 --- /dev/null +++ b/dot-net/UnitTesting/WriteUnitTest/Repositories/IModuleRepository.cs @@ -0,0 +1,9 @@ +using WriteUnitTest.Entities; + +namespace WriteUnitTest.Repositories +{ + public interface IModuleRepository + { + Module GetModule(int lessonId); + } +} \ No newline at end of file diff --git a/dot-net/UnitTesting/WriteUnitTest/Repositories/LessonRepository.cs b/dot-net/UnitTesting/WriteUnitTest/Repositories/LessonRepository.cs index e122d605..ae6acf90 100644 --- a/dot-net/UnitTesting/WriteUnitTest/Repositories/LessonRepository.cs +++ b/dot-net/UnitTesting/WriteUnitTest/Repositories/LessonRepository.cs @@ -4,7 +4,7 @@ namespace WriteUnitTest.Repositories { - public class LessonRepository + public class LessonRepository : ILessonRepository { private readonly List lessonList; diff --git a/dot-net/UnitTesting/WriteUnitTest/Repositories/ModuleRepository.cs b/dot-net/UnitTesting/WriteUnitTest/Repositories/ModuleRepository.cs index 5dacaede..5c2014fb 100644 --- a/dot-net/UnitTesting/WriteUnitTest/Repositories/ModuleRepository.cs +++ b/dot-net/UnitTesting/WriteUnitTest/Repositories/ModuleRepository.cs @@ -4,7 +4,7 @@ namespace WriteUnitTest.Repositories { - public class ModuleRepository + public class ModuleRepository : IModuleRepository { private readonly List moduleList; diff --git a/dot-net/UnitTesting/WriteUnitTest/Services/LessonService.cs b/dot-net/UnitTesting/WriteUnitTest/Services/LessonService.cs index e0f04a5b..f8a3e9aa 100644 --- a/dot-net/UnitTesting/WriteUnitTest/Services/LessonService.cs +++ b/dot-net/UnitTesting/WriteUnitTest/Services/LessonService.cs @@ -4,21 +4,31 @@ namespace WriteUnitTest.Services { public class LessonService { + private readonly ILessonRepository LessonRepository; + private readonly IModuleRepository ModuleRepository; + + public LessonService() { + LessonRepository = new LessonRepository(); + ModuleRepository = new ModuleRepository(); + } + + public LessonService(ILessonRepository lessonRepository, IModuleRepository moduleRepository) + { + LessonRepository = lessonRepository; + ModuleRepository = moduleRepository; } public void UpdateLessonGrade(int lessonId, double grade) { - var lessonRepo = new LessonRepository(); - var lesson = lessonRepo.GetLesson(lessonId); + var lesson = LessonRepository.GetLesson(lessonId); lesson.Grade = grade; if (!lesson.IsPassed) { - var moduleRepository = new ModuleRepository(); - var module = moduleRepository.GetModule(lessonId); + var module = ModuleRepository.GetModule(lessonId); if (grade >= module.MinimumPassingGrade) { diff --git a/dot-net/UnitTesting/WriteUnitTest/WriteUnitTest.csproj b/dot-net/UnitTesting/WriteUnitTest/WriteUnitTest.csproj index 642aa27b..b682bcaf 100644 --- a/dot-net/UnitTesting/WriteUnitTest/WriteUnitTest.csproj +++ b/dot-net/UnitTesting/WriteUnitTest/WriteUnitTest.csproj @@ -47,6 +47,8 @@ + +