-
Notifications
You must be signed in to change notification settings - Fork 0
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
start exam implementation: with the notate method, driven by test #5
base: develop
Are you sure you want to change the base?
Changes from 8 commits
fe30edd
b284f44
a0589aa
7797654
bf9fbbb
364a213
e400624
59b25bb
403c540
dd61d5b
6d4b919
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
distributionBase=GRADLE_USER_HOME | ||
distributionPath=wrapper/dists | ||
distributionUrl=https\://services.gradle.org/distributions/gradle-4.8-bin.zip | ||
zipStoreBase=GRADLE_USER_HOME | ||
zipStorePath=wrapper/dists | ||
distributionUrl=https\://services.gradle.org/distributions/gradle-4.4-bin.zip |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
package com.students.results.entities | ||
|
||
import arrow.core.Either | ||
import java.math.BigDecimal | ||
|
||
data class Exam(val id: Long) { | ||
fun validateNotation(notation: BigDecimal): Either<InvalidNotationForThisExamException, Unit> = | ||
when (notation) { | ||
in (0.toBigDecimal()..20.toBigDecimal()) -> Either.right(Unit) | ||
else -> Either.left(InvalidNotationForThisExamException("This notation, $notation, is not in bound")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd probably call it a GradeOutOfBoundsException |
||
} | ||
} | ||
|
||
class InvalidNotationForThisExamException(msg: String?) : RuntimeException(msg) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kotlin exceptions are unchecked, so simply Exception is sufficient unless cross-compatibility with Java is required. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
package com.students.results.entities | ||
|
||
import arrow.core.Either | ||
import java.math.BigDecimal | ||
|
||
typealias Notes = Map<Exam, BigDecimal> | ||
|
||
data class Student(val id: Long, val notes: Notes = emptyMap()) { | ||
fun getNotation(exam: Exam): Either<NotEvaluatedException, BigDecimal> = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oops ! get grade ! |
||
when { | ||
notes.containsKey(exam) -> Either.right(notes.getValue(exam)) | ||
else -> Either.left(NotEvaluatedException()) | ||
} | ||
|
||
fun notate(exam: Exam, note: BigDecimal): Either<InvalidNotationForThisExamException, Student> = | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we grade an exam in English :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oops should be grade not notate There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And a Grade not a BigDecimal |
||
exam.validateNotation(note).map { | ||
copy(id = id, | ||
notes = notes + mapOf(exam to note)) | ||
} | ||
} | ||
|
||
class NotEvaluatedException(msg: String? = null, throwable: Throwable? = null) : RuntimeException(msg, throwable) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A default message is probably preferable to null, yes? |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
package com.students.results.interactors | ||
|
||
import com.students.results.repository.ExamsRepository | ||
import com.students.results.services.Exams | ||
|
||
class ExamsInteractor(private val examsRepository: ExamsRepository) : Exams |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
package com.students.results.interactors | ||
|
||
import arrow.core.Either | ||
import arrow.core.flatMap | ||
import com.students.results.repository.ExamsRepository | ||
import com.students.results.repository.StudentsRepository | ||
import com.students.results.services.NotateExamException | ||
import com.students.results.services.Students | ||
import com.students.results.services.requests.NotateExam | ||
|
||
class StudentsInteractor(private val studentsRepository: StudentsRepository, private val examsRepository: ExamsRepository) : Students { | ||
|
||
override fun notate(notateExam: NotateExam): Either<NotateExamException, Unit> = | ||
studentsRepository.findStudentById(studentId = 1).flatMap { student -> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. your studentId is defined in the notateExam class, we should probably use that otherwise your student 1 will get all the grades There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I already noticed it in my comment, but thx |
||
examsRepository.findExamById(examId = notateExam.examId).flatMap { exam -> | ||
student.notate(exam, notateExam.note).flatMap { updatedStudent -> | ||
studentsRepository.save(updatedStudent) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are too many levels of abstraction here and it is not clear what this method is doing. Remember the SRP :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What would you remove there is only two call and entity method. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe by giving up either, but I want to use it to handle errors There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mostly that we jump from a student into an exam into a grade for that exam and student. all the while nesting. Exam doesn't depend on student, so the nesting is odd. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nesting helps to get the values directly. The other solution is to do if(isRight()=> toOption().get()) etc..etc but I really don't like that solution (I tested it) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I searched a lot, and normally the following code is better and compile (I need to check at home) notateExam.also {
ForEither<RepositoryException>() extensions {
tupled(
studentsRepository.findStudentById(studentId),
examsRepository.findExamById(examId)
)
}.flatMap { (student, exam) ->
student.notate(exam, note).flatMap { studentsRepository.save(it) }
}.mapLeft { NotateExamException(throwable = it) }
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does not work, please close it, it is useless |
||
} | ||
} | ||
}.mapLeft { NotateExamException(throwable = it) } | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
package com.students.results.repository | ||
|
||
import arrow.core.Either | ||
import com.students.results.entities.Exam | ||
|
||
interface ExamsRepository { | ||
|
||
fun findExamById(examId: Long): Either<NotFoundException, Exam> | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package com.students.results.repository | ||
|
||
|
||
sealed class RepositoryException(msg: String? = null, throwable: Throwable? = null) : RuntimeException(msg, throwable) | ||
|
||
class NotFoundException(msg: String? = null, throwable: Throwable? = null) : RepositoryException(msg, throwable) | ||
class NotWrittenException(msg: String? = null, throwable: Throwable? = null) : RepositoryException(msg, throwable) | ||
class RepositoryNotAvailableException(msg: String? = null, throwable: Throwable? = null) : RepositoryException(msg, throwable) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
package com.students.results.repository | ||
|
||
import arrow.core.Either | ||
import com.students.results.entities.Student | ||
|
||
interface StudentsRepository { | ||
|
||
fun findStudentById(studentId: Long): Either<RepositoryException, Student> | ||
fun save(student: Student): Either<RepositoryException, Unit> | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
package com.students.results.services | ||
|
||
interface Exams { | ||
} | ||
|
||
class NotateExamException(message: String? = null, throwable: Throwable? = null) : RuntimeException(message, throwable) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
package services | ||
|
||
sealed class ServicesException(msg: String? = null, throwable: Throwable? = null) : RuntimeException(msg, throwable) | ||
|
||
class NotFoundException(msg: String? = null, throwable: Throwable? = null) : ServicesException(msg, throwable) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
package com.students.results.services | ||
|
||
import arrow.core.Either | ||
import com.students.results.services.requests.NotateExam | ||
|
||
interface Students { | ||
|
||
fun notate(notateExam: NotateExam): Either<NotateExamException, Unit> | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
package com.students.results.services.requests | ||
|
||
import java.math.BigDecimal | ||
|
||
data class NotateExam(val examId: Long, val studentId: Long, val note: BigDecimal) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
package entities | ||
|
||
import com.students.results.entities.Exam | ||
import com.students.results.entities.Student | ||
import org.amshove.kluent.shouldBe | ||
import org.amshove.kluent.shouldEqual | ||
import org.jetbrains.spek.api.Spek | ||
import org.jetbrains.spek.api.dsl.given | ||
import org.jetbrains.spek.api.dsl.it | ||
|
||
class StudentTest : Spek({ | ||
|
||
given("a student") { | ||
val student = Student(id = 40L) | ||
val exam = Exam(id = 50L) | ||
|
||
it("getNot without note should returns NotEvaluatedException") { | ||
student.getNotation(exam).isLeft() shouldBe true | ||
} | ||
it("notate an exam with 20 and get notation should return 20") { | ||
student.notate(exam, "20".toBigDecimal()).apply { | ||
isRight() shouldBe true | ||
map { | ||
it.getNotation(exam).apply { | ||
isRight() shouldBe true | ||
map { it shouldEqual "20".toBigDecimal() } | ||
} | ||
} | ||
} | ||
} | ||
it("notate less than zero should fail to register note") { | ||
student.notate(exam, "-1".toBigDecimal()).isLeft() shouldBe true | ||
} | ||
} | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package com.students.results.interactors | ||
|
||
import org.jetbrains.spek.api.Spek | ||
|
||
class ExamsInteractorTest : Spek({ | ||
|
||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
package com.students.results.interactors | ||
|
||
import arrow.core.Either | ||
import com.students.results.entities.Exam | ||
import com.students.results.entities.Student | ||
import com.students.results.repository.ExamsRepository | ||
import com.students.results.repository.StudentsRepository | ||
import com.students.results.services.requests.NotateExam | ||
import io.mockk.every | ||
import io.mockk.mockk | ||
import io.mockk.slot | ||
import io.mockk.verify | ||
import org.amshove.kluent.`should be` | ||
import org.amshove.kluent.shouldBe | ||
import org.amshove.kluent.shouldEqual | ||
import org.jetbrains.spek.api.Spek | ||
import org.jetbrains.spek.api.dsl.given | ||
import org.jetbrains.spek.api.dsl.it | ||
|
||
|
||
class StudentsInteractorTest : Spek({ | ||
|
||
val student = Student(id = 10L) | ||
given("a student interactor") { | ||
|
||
val studentSlot = slot<Student>() | ||
val studentRepository = mockk<StudentsRepository>() { | ||
every { findStudentById(1) } returns Either.right(student) | ||
every { save(student = capture(studentSlot)) } returns Either.right(Unit) | ||
} | ||
val exam = Exam(id = 5) | ||
val examsRepository = mockk<ExamsRepository>().apply { | ||
every { findExamById(5) } returns Either.right(exam) | ||
} | ||
val studentsInteractor = StudentsInteractor(studentRepository, examsRepository) | ||
studentsInteractor.notate20().apply { | ||
it("should notate an existing exam with a note of 20") { | ||
isRight() `should be` true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The actual grade isn't checked here. How do we know it's 20? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes this test should be deleted because check is done in the last test |
||
} | ||
it("should retrieve student by his id") { | ||
verify { studentRepository.findStudentById(1) } | ||
} | ||
it("should retrieve exam by id") { | ||
verify { examsRepository.findExamById(5) } | ||
} | ||
it("should save student to the repository with the expected notation of 20 for this exam") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Grade not notation |
||
studentSlot.captured.getNotation(exam).apply { | ||
isRight() shouldBe true | ||
map { it shouldEqual "20".toBigDecimal() } | ||
} | ||
} | ||
} | ||
} | ||
}) | ||
|
||
private fun StudentsInteractor.notate20() = notate(NotateExam( | ||
examId = 5, | ||
studentId = 1, | ||
note = "20".toBigDecimal() | ||
)) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,18 @@ | ||
|
||
apply plugin: 'kotlin-spring' | ||
|
||
dependencies { | ||
compile project(':business-rules') | ||
compile project(':interactors') | ||
compile group: 'org.springframework.data', name: 'spring-data-redis', version: '2.0.7.RELEASE' | ||
testCompile 'it.ozimov:embedded-redis:0.7.2' | ||
testCompile group: 'org.springframework', name: 'spring-test', version: '5.0.6.RELEASE' | ||
testImplementation( | ||
'org.junit.jupiter:junit-jupiter-api:5.1.0' | ||
) | ||
testRuntimeOnly( | ||
'org.junit.jupiter:junit-jupiter-engine:5.1.0' | ||
) | ||
} | ||
|
||
test { | ||
useJUnitPlatform() | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
package com.students.results.redis.configuration | ||
|
||
import org.springframework.context.annotation.Bean | ||
import org.springframework.context.annotation.Configuration | ||
import org.springframework.data.redis.connection.jedis.JedisConnectionFactory | ||
import org.springframework.data.redis.core.RedisTemplate | ||
import org.springframework.data.redis.repository.configuration.EnableRedisRepositories | ||
|
||
@Configuration | ||
@EnableRedisRepositories | ||
class RedisConfiguration { | ||
|
||
@Bean | ||
fun connectionFactory() = JedisConnectionFactory() | ||
|
||
@Bean | ||
fun redisTemplate(jedisConnectionFactory: JedisConnectionFactory) = RedisTemplate<Any, Any>().apply { | ||
connectionFactory = jedisConnectionFactory | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
package embeded.redis.startup | ||
|
||
import com.students.results.redis.configuration.RedisConfiguration | ||
import org.junit.jupiter.api.AfterAll | ||
import org.junit.jupiter.api.BeforeAll | ||
import org.junit.jupiter.api.Test | ||
import org.junit.jupiter.api.TestInstance | ||
import org.junit.jupiter.api.TestInstance.* | ||
import org.junit.runner.RunWith | ||
import org.springframework.beans.factory.annotation.Autowired | ||
import org.springframework.test.context.ContextConfiguration | ||
import org.springframework.test.context.junit4.SpringRunner | ||
import redis.embedded.RedisServer | ||
|
||
@RunWith(SpringRunner::class) | ||
@ContextConfiguration(classes = [RedisConfiguration::class, RedisEmbededConfiguration::class]) | ||
@TestInstance(Lifecycle.PER_CLASS) | ||
class EmbededRedisTest { | ||
|
||
@Autowired | ||
private lateinit var redisServer: RedisServer | ||
|
||
@BeforeAll | ||
fun beforeStart() { | ||
redisServer.start() | ||
} | ||
|
||
@Test | ||
fun runATEst(){ | ||
println("I am running") | ||
} | ||
|
||
@AfterAll | ||
fun afterStart() { | ||
redisServer.stop() | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
package embeded.redis.startup | ||
|
||
import org.springframework.context.annotation.Bean | ||
import org.springframework.context.annotation.Configuration | ||
import redis.embedded.RedisServer | ||
|
||
@Configuration | ||
class RedisEmbededConfiguration { | ||
|
||
@Bean | ||
fun redisServer(): RedisServer = RedisServer(6379) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
rootProject.name = 'students-results' | ||
|
||
include 'business-rules', 'main', 'persistence' | ||
include 'interactors', 'main', 'persistence' | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In English, this would be a grade :)