Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interview assignment completed #281

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,10 @@ build/
# Package Files #
*.jar

*.log
*.log
/.vs
/dot-net/UnitTesting
/dot-net/WhatWouldYouChange/obj
/dot-net/WhatWouldYouChange/bin/Debug
/dot-net/WhatWouldYouChange/.vs/ExampleProgram/v15
/dot-net/.vs
Original file line number Diff line number Diff line change
@@ -1,13 +1,100 @@
using Microsoft.VisualStudio.TestTools.UnitTesting;
using FizzWare.NBuilder;
using Moq;
using NUnit.Framework;
using WriteUnitTest.Entities;
using WriteUnitTest.Interfaces;
using WriteUnitTest.Services;

namespace WriteUnitTest.UnitTests.Services
{
[TestClass]
[TestFixture]
public class LessonServiceUnitTests
{
[TestMethod]
public void UpdateLessonGrade_Test()
[Test]
public void UpdateLessonGrade_LessonReturnedIsPassed_ShouldSetLessonIsPassedToTrue()
{
//Arrange
var mockLessonRepository = new Mock<ILessonRepository>();
var mockModuleRepository = new Mock<IModuleRepository>();
var service = new LessonService(mockLessonRepository.Object, mockModuleRepository.Object);
//var testLesson = Builder<Lesson>.CreateNew().Build();
Lesson testLesson = new Lesson {
LessonId = 1,
Grade = 70.5,
IsPassed = true
};
mockLessonRepository.Setup(q => q.GetLesson(It.IsAny<int>())).Returns(testLesson);

//Act
var lesson = service.UpdateLessonGrade(testLesson.LessonId, 80.5);

//Assert
NUnit.Framework.Assert.That(lesson, Is.Not.Null);
Assert.That(lesson.IsPassed, Is.True);
mockLessonRepository.Verify(x => x.GetLesson(It.Is<int>(q=>q==testLesson.LessonId)), Times.Once, "Missing mandatory call to lesson repository to get lesson.");
mockModuleRepository.Verify(x => x.GetModule(It.IsAny<int>()), Times.Never, "Invalid call made to module repository to get module.");
}

[Test]
public void UpdateLessonGrade_LessonReturnedIsFailed_ShouldSetLessonIsPassedToFalse()
{
//Arrange
var mockLessonRepository = new Mock<ILessonRepository>();
var mockModuleRepository = new Mock<IModuleRepository>();
var service = new LessonService(mockLessonRepository.Object, mockModuleRepository.Object);
//var testLesson = Builder<Lesson>.CreateNew().Build();
Lesson testLesson = new Lesson
{
LessonId = 1,
Grade = 70.5,
IsPassed = false
};
mockLessonRepository.Setup(q => q.GetLesson(It.IsAny<int>())).Returns(testLesson);
Module testModule = new Module
{
MinimumPassingGrade = 75
};
mockModuleRepository.Setup(q => q.GetModule(It.IsAny<int>())).Returns(testModule);

//Act
var lesson = service.UpdateLessonGrade(testLesson.LessonId, 74);

//Assert
NUnit.Framework.Assert.That(lesson, Is.Not.Null);
Assert.That(lesson.IsPassed, Is.False);
mockLessonRepository.Verify(x => x.GetLesson(It.Is<int>(q => q == testLesson.LessonId)), Times.Once, "Missing mandatory call to lesson repository to get lesson.");
mockModuleRepository.Verify(x => x.GetModule(It.IsAny<int>()), Times.Once, "Missing mandatory call to module repository to get module.");
}

[Test]
public void UpdateLessonGrade_LessonReturnedIsFailed_ShouldSetLessonIsPassedToTrue()
{
//Arrange
var mockLessonRepository = new Mock<ILessonRepository>();
var mockModuleRepository = new Mock<IModuleRepository>();
var service = new LessonService(mockLessonRepository.Object, mockModuleRepository.Object);
//var testLesson = Builder<Lesson>.CreateNew().Build();
Lesson testLesson = new Lesson
{
LessonId = 1,
Grade = 70.5,
IsPassed = false
};
mockLessonRepository.Setup(q => q.GetLesson(It.IsAny<int>())).Returns(testLesson);
Module testModule = new Module
{
MinimumPassingGrade = 70
};
mockModuleRepository.Setup(q => q.GetModule(It.IsAny<int>())).Returns(testModule);

//Act
var lesson = service.UpdateLessonGrade(testLesson.LessonId, 74);

//Assert
NUnit.Framework.Assert.That(lesson, Is.Not.Null);
Assert.That(lesson.IsPassed, Is.True);
mockLessonRepository.Verify(x => x.GetLesson(It.Is<int>(q => q == testLesson.LessonId)), Times.Once, "Missing mandatory call to lesson repository to get lesson.");
mockModuleRepository.Verify(x => x.GetModule(It.IsAny<int>()), Times.Once, "Missing mandatory call to module repository to get module.");
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Import Project="..\packages\NUnit.3.11.0\build\NUnit.props" Condition="Exists('..\packages\NUnit.3.11.0\build\NUnit.props')" />
<PropertyGroup>
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
Expand All @@ -16,6 +17,8 @@
<ReferencePath>$(ProgramFiles)\Common Files\microsoft shared\VSTT\$(VisualStudioVersion)\UITestExtensionPackages</ReferencePath>
<IsCodedUITest>False</IsCodedUITest>
<TestProjectType>UnitTest</TestProjectType>
<NuGetPackageImportStamp>
</NuGetPackageImportStamp>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
<DebugSymbols>true</DebugSymbols>
Expand All @@ -35,7 +38,26 @@
<WarningLevel>4</WarningLevel>
</PropertyGroup>
<ItemGroup>
<Reference Include="Castle.Core, Version=4.0.0.0, Culture=neutral, PublicKeyToken=407dd0808d44fbdc, processorArchitecture=MSIL">
<HintPath>..\packages\Castle.Core.4.3.1\lib\net45\Castle.Core.dll</HintPath>
</Reference>
<Reference Include="FizzWare.NBuilder, Version=1.0.0.0, Culture=neutral, PublicKeyToken=5651b03e12e42c12, processorArchitecture=MSIL">
<HintPath>..\packages\NBuilder.6.0.0\lib\net40\FizzWare.NBuilder.dll</HintPath>
</Reference>
<Reference Include="Moq, Version=4.10.0.0, Culture=neutral, PublicKeyToken=69f491c39445e920, processorArchitecture=MSIL">
<HintPath>..\packages\Moq.4.10.1\lib\net45\Moq.dll</HintPath>
</Reference>
<Reference Include="nunit.framework, Version=3.11.0.0, Culture=neutral, PublicKeyToken=2638cd05610744eb, processorArchitecture=MSIL">
<HintPath>..\packages\NUnit.3.11.0\lib\net45\nunit.framework.dll</HintPath>
</Reference>
<Reference Include="System" />
<Reference Include="System.Configuration" />
<Reference Include="System.Runtime.CompilerServices.Unsafe, Version=4.0.4.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL">
<HintPath>..\packages\System.Runtime.CompilerServices.Unsafe.4.5.0\lib\netstandard2.0\System.Runtime.CompilerServices.Unsafe.dll</HintPath>
</Reference>
<Reference Include="System.Threading.Tasks.Extensions, Version=4.2.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51, processorArchitecture=MSIL">
<HintPath>..\packages\System.Threading.Tasks.Extensions.4.5.1\lib\netstandard2.0\System.Threading.Tasks.Extensions.dll</HintPath>
</Reference>
</ItemGroup>
<Choose>
<When Condition="('$(VisualStudioVersion)' == '10.0' or '$(VisualStudioVersion)' == '') and '$(TargetFrameworkVersion)' == 'v3.5'">
Expand All @@ -55,6 +77,15 @@
<Compile Include="Services\LessonServiceUnitTests.cs" />
<Compile Include="Properties\AssemblyInfo.cs" />
</ItemGroup>
<ItemGroup>
<None Include="packages.config" />
</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>
Expand All @@ -75,6 +106,12 @@
</Choose>
<Import Project="$(VSToolsPath)\TeamTest\Microsoft.TestTools.targets" Condition="Exists('$(VSToolsPath)\TeamTest\Microsoft.TestTools.targets')" />
<Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
<Target Name="EnsureNuGetPackageBuildImports" BeforeTargets="PrepareForBuild">
<PropertyGroup>
<ErrorText>This project references NuGet package(s) that are missing on this computer. Use NuGet Package Restore to download them. For more information, see http://go.microsoft.com/fwlink/?LinkID=322105. The missing file is {0}.</ErrorText>
</PropertyGroup>
<Error Condition="!Exists('..\packages\NUnit.3.11.0\build\NUnit.props')" Text="$([System.String]::Format('$(ErrorText)', '..\packages\NUnit.3.11.0\build\NUnit.props'))" />
</Target>
<!-- To modify your build process, add your task inside one of the targets below and uncomment it.
Other similar extension points exist, see Microsoft.Common.targets.
<Target Name="BeforeBuild">
Expand Down
14 changes: 9 additions & 5 deletions dot-net/UnitTesting/WriteUnitTest/Program.cs
Original file line number Diff line number Diff line change
@@ -1,18 +1,22 @@
using WriteUnitTest.Services;
using System;
using WriteUnitTest.Services;

namespace WriteUnitTest
{
public class Program
{
public static void Main(string[] args)
{
var lessonSvc = new LessonService();
//var lessonSvc = new LessonService();

var lessonId = 12;
//var lessonId = 12;

var grade = 98.2d;
//var grade = 98.2d;

lessonSvc.UpdateLessonGrade(lessonId, grade);
//lessonSvc.UpdateLessonGrade(lessonId, grade);

//Run unit test via Test Explorer
Console.ReadLine();
}
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
using System.Collections.Generic;
using System.Linq;
using WriteUnitTest.Entities;
using WriteUnitTest.Interfaces;

namespace WriteUnitTest.Repositories
{
public class LessonRepository
public class LessonRepository: ILessonRepository
{
private readonly List<Lesson> lessonList;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
using System.Collections.Generic;
using System.Linq;
using WriteUnitTest.Entities;
using WriteUnitTest.Interfaces;

namespace WriteUnitTest.Repositories
{
public class ModuleRepository
public class ModuleRepository: IModuleRepository
{
private readonly List<Module> moduleList;

Expand Down
20 changes: 13 additions & 7 deletions dot-net/UnitTesting/WriteUnitTest/Services/LessonService.cs
Original file line number Diff line number Diff line change
@@ -1,24 +1,28 @@
using WriteUnitTest.Repositories;
using WriteUnitTest.Entities;
using WriteUnitTest.Interfaces;

namespace WriteUnitTest.Services
{
public class LessonService
{
public LessonService()
private readonly ILessonRepository _lessonRepository;
private readonly IModuleRepository _moduleRepository;

public LessonService(ILessonRepository lessonRepository, IModuleRepository moduleRepository)
{
_lessonRepository = lessonRepository;
_moduleRepository = moduleRepository;
}

public void UpdateLessonGrade(int lessonId, double grade)
public Lesson 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)
{
Expand All @@ -29,6 +33,8 @@ public void UpdateLessonGrade(int lessonId, double grade)
lesson.IsPassed = false;
}
}

return lesson;
}
}
}
2 changes: 2 additions & 0 deletions dot-net/UnitTesting/WriteUnitTest/WriteUnitTest.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
<ItemGroup>
<Compile Include="Entities\Lesson.cs" />
<Compile Include="Entities\Module.cs" />
<Compile Include="Interfaces\ILessonRepository.cs" />
<Compile Include="Interfaces\IModuleRepository.cs" />
<Compile Include="Program.cs" />
<Compile Include="Properties\AssemblyInfo.cs" />
<Compile Include="Repositories\LessonRepository.cs" />
Expand Down
Original file line number Diff line number Diff line change
@@ -1 +1,7 @@
C:\Users\lyncha1\SourceCode\interviews\dot-net\UnitTesting\WriteUnitTest\bin\Debug\WriteUnitTest.exe.config
C:\SourceCode\ElsevierInterview\dot-net\UnitTesting\WriteUnitTest\obj\Debug\WriteUnitTest.csproj.CoreCompileInputs.cache
C:\SourceCode\ElsevierInterview\dot-net\UnitTesting\WriteUnitTest\bin\Debug\WriteUnitTest.exe.config
C:\SourceCode\ElsevierInterview\dot-net\UnitTesting\WriteUnitTest\bin\Debug\WriteUnitTest.exe
C:\SourceCode\ElsevierInterview\dot-net\UnitTesting\WriteUnitTest\bin\Debug\WriteUnitTest.pdb
C:\SourceCode\ElsevierInterview\dot-net\UnitTesting\WriteUnitTest\obj\Debug\WriteUnitTest.exe
C:\SourceCode\ElsevierInterview\dot-net\UnitTesting\WriteUnitTest\obj\Debug\WriteUnitTest.pdb
23 changes: 22 additions & 1 deletion dot-net/WhatWouldYouChange/INSTRUCTIONS.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,27 @@
you think should be made in the section below. Don't spend more than 30 minutes on this entire assignment.
Feel free to describe why you made particular design decisions.


Suggested Changes:
1. In the entry point of the app - Program -> Main,
1.1 The file name is hardcoded and has a typo.
1.2 Instead of hardcoding, FileName can be passed in as argument (string[] args)
2. In the method ExampleClass.ExampleMethod,
2.1 There is no check for whether the FileExists. I understand there is try-catch to handle any exception. However, if we add the check and raise or log errors, the code will be much cleaner and easy to maintain.
2.2. StreamReader is used to read the contents of the file. However there is no use of using syntax. Thus it look like resource will not be disposed properly. Moreover I dont see the File.Close method call to close the file.
2.3 Instead of StreamReader, File.ReadAllText can be used. The code is less verbose this way and it will take care of disposing resources automatically. It will not be developer's headache to worry about that anymore.
2.4 The code assumes the file with provide filename is will exists in current run-time working directory (say bin folder) which is not a good assumption. There can be various approaches take to fix this - either have fully qualified path or UNC path or a config param (app.config appSettings)
to store the source directory. Then use Path.Combine to get full path and use it to read the contents of the file.
2.5 In the catch block the default text string is built using concatenation approach. Instead, we can use verbatim string i.e. var s = @"This is line 1
This is line 2
This is line 3
.
.
.
This is line n";
We can also use StringBuilder instead.
3. Design Patterns
3.1 Repository Pattern & Dependency Injection -We can define interface instead of using concrete implementation (ExampleClass). This way we can easy swap underlying way of getting the file contents. Say down the lane we get the contents from say api stream instead of file stream.
This approach will also make our code unit testable. We can easily mock against the interface and test our method or unit of code.
3.2 Factory Pattern & Inversion of Control Container - Instead of instantiating concrete class instance, we can have IoC container inject/spin-up the concreate objects into our code.