Stephen Picone's dot-net techinical test submission

dot-net/UnitTesting/WriteUnitTest.UnitTests/Services/LessonServiceUnitTests.cs

@@ -1,13 +1,120 @@
 using Microsoft.VisualStudio.TestTools.UnitTesting;
+using WriteUnitTest.Services;
 
 namespace WriteUnitTest.UnitTests.Services
 {
     [TestClass]
     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");
         }
     }
 }

dot-net/UnitTesting/WriteUnitTest.UnitTests/WriteUnitTest.UnitTests.csproj

@@ -55,6 +55,12 @@
     <Compile Include="Services\LessonServiceUnitTests.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
   </ItemGroup>
+  <ItemGroup>
+    <ProjectReference Include="..\WriteUnitTest\WriteUnitTest.csproj">
+      <Project>{00A40A05-8314-4F25-A444-46DDEAC3497E}</Project>
+      <Name>WriteUnitTest</Name>
+    </ProjectReference>
+  </ItemGroup>
   <Choose>
     <When Condition="'$(VisualStudioVersion)' == '10.0' And '$(IsCodedUITest)' == 'True'">
       <ItemGroup>

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);
         }

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
                 }
             };
         }

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; }
+
+        /// <summary>
+        /// 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.
+        /// </summary>
+        /// <param name="lessonId">The ID of the lesson to be updated</param>
+        /// <param name="grade">The grade for the lesson</param>
         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)
                 {

dot-net/WhatWouldYouChange/INSTRUCTIONS.txt

@@ -5,4 +5,17 @@
 
 
 Suggested Changes:
-
+	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.	
+