From a95d910607c88566159cb5c5cbaee803bebb69de Mon Sep 17 00:00:00 2001 From: John Duffell Date: Mon, 3 Aug 2020 17:38:04 +0100 Subject: [PATCH 1/2] don't use apply method in gift code generator --- .../redemption/generator/GiftCodeGenerator.scala | 16 ++++++++-------- .../generator/GiftCodeGeneratorSpec.scala | 14 +++++++------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/support-services/src/main/scala/com/gu/support/redemption/generator/GiftCodeGenerator.scala b/support-services/src/main/scala/com/gu/support/redemption/generator/GiftCodeGenerator.scala index 048aeea062..e06ba62b4b 100644 --- a/support-services/src/main/scala/com/gu/support/redemption/generator/GiftCodeGenerator.scala +++ b/support-services/src/main/scala/com/gu/support/redemption/generator/GiftCodeGenerator.scala @@ -2,18 +2,18 @@ package com.gu.support.redemption.generator import java.security.SecureRandom -import com.gu.support.redemption.generator.ConstructCode.GenerateGiftCode +import com.gu.support.redemption.generator.CodeBuilder.GenerateGiftCode object GiftCodeGenerator { lazy val randomGiftCodes: Iterator[GenerateGiftCode] = { val gen = new SecureRandom() val ints = Iterator.continually(gen.nextInt()) - apply(ints) + fromRandom(ints) } - def apply(random: Iterator[Int]): Iterator[GenerateGiftCode] = - CodeSuffixGenerator(random).map(ConstructCode.apply) + def fromRandom(random: Iterator[Int]): Iterator[GenerateGiftCode] = + CodeSuffixGenerator.generate(random).map(CodeBuilder.build) } @@ -29,7 +29,7 @@ object GiftDuration { } -object ConstructCode { +object CodeBuilder { private val prefix = "gd" @@ -46,7 +46,7 @@ object ConstructCode { def withDuration(duration: GiftDuration): GiftCode } - def apply(code: CodeSuffixGenerator.CodeSuffix): GenerateGiftCode = + def build(code: CodeSuffixGenerator.CodeSuffix): GenerateGiftCode = (duration: GiftDuration) => { val init = prefix + duration.code + "-" GiftCode(init + code.value).get @@ -64,10 +64,10 @@ object CodeSuffixGenerator { .map(new CodeSuffix(_)) } - def apply(random: Iterator[Int]): Iterator[CodeSuffix] = + def generate(random: Iterator[Int]): Iterator[CodeSuffix] = random.grouped(6).map(codeFromGroup).map(CodeSuffix.apply).map(_.get) - def codeFromGroup(groupedInts: Seq[Int]): String = { + private[generator] def codeFromGroup(groupedInts: Seq[Int]): String = { val chars = groupedInts .map(int => java.lang.Integer.toString(int, 34).last) .map { diff --git a/support-services/src/test/scala/com/gu/support/redemption/generator/GiftCodeGeneratorSpec.scala b/support-services/src/test/scala/com/gu/support/redemption/generator/GiftCodeGeneratorSpec.scala index 72e9e2498c..8acc8879dc 100644 --- a/support-services/src/test/scala/com/gu/support/redemption/generator/GiftCodeGeneratorSpec.scala +++ b/support-services/src/test/scala/com/gu/support/redemption/generator/GiftCodeGeneratorSpec.scala @@ -6,7 +6,7 @@ import org.scalatest.matchers.should.Matchers class GiftCodeGeneratorSpec extends AnyFlatSpec with Matchers { it should "work in the basic case" in { - val giftCode = GiftCodeGenerator(Iterator.continually(0)).next.withDuration(GiftDuration.Gift3Month) + val giftCode = GiftCodeGenerator.fromRandom(Iterator.continually(0)).next.withDuration(GiftDuration.Gift3Month) giftCode.value should be("gd03-000000") } @@ -22,16 +22,16 @@ class GiftCodeGeneratorSpec extends AnyFlatSpec with Matchers { } -class ConstructCodeSpec extends AnyFlatSpec with Matchers { +class CodeBuilderSpec extends AnyFlatSpec with Matchers { it should "get the durations right for the codes" in { - ConstructCode(CodeSuffixGenerator.CodeSuffix("000000").get).withDuration(GiftDuration.Gift3Month).value should be("gd03-000000") - ConstructCode(CodeSuffixGenerator.CodeSuffix("000000").get).withDuration(GiftDuration.Gift6Month).value should be("gd06-000000") - ConstructCode(CodeSuffixGenerator.CodeSuffix("000000").get).withDuration(GiftDuration.Gift12Month).value should be("gd12-000000") + CodeBuilder.build(CodeSuffixGenerator.CodeSuffix("000000").get).withDuration(GiftDuration.Gift3Month).value should be("gd03-000000") + CodeBuilder.build(CodeSuffixGenerator.CodeSuffix("000000").get).withDuration(GiftDuration.Gift6Month).value should be("gd06-000000") + CodeBuilder.build(CodeSuffixGenerator.CodeSuffix("000000").get).withDuration(GiftDuration.Gift12Month).value should be("gd12-000000") } it should "not allow invalid codes to be constructed" in { - ConstructCode.GiftCode("invalid") should be(None) + CodeBuilder.GiftCode("invalid") should be(None) } } @@ -48,7 +48,7 @@ class CodeSuffixGeneratorSpec extends AnyFlatSpec with Matchers { it should "cycle through all the possiblities" in { val seq = Stream.from(0).iterator - CodeSuffixGenerator(seq).take(7).toList.map(_.value) should be( + CodeSuffixGenerator.generate(seq).take(7).toList.map(_.value) should be( List("0y2345", "6789ab", "cdefgh", "ijkzmn", "opqrst", "uvwx0y", "234567") ) } From e56bd86495e52f9d85122f863d2e9c26e10d9251 Mon Sep 17 00:00:00 2001 From: John Duffell Date: Tue, 4 Aug 2020 13:12:48 +0100 Subject: [PATCH 2/2] naming - generate gift codes from ints, they may not be random --- .../gu/support/redemption/generator/GiftCodeGenerator.scala | 4 ++-- .../support/redemption/generator/GiftCodeGeneratorSpec.scala | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/support-services/src/main/scala/com/gu/support/redemption/generator/GiftCodeGenerator.scala b/support-services/src/main/scala/com/gu/support/redemption/generator/GiftCodeGenerator.scala index e06ba62b4b..03900fff98 100644 --- a/support-services/src/main/scala/com/gu/support/redemption/generator/GiftCodeGenerator.scala +++ b/support-services/src/main/scala/com/gu/support/redemption/generator/GiftCodeGenerator.scala @@ -9,10 +9,10 @@ object GiftCodeGenerator { lazy val randomGiftCodes: Iterator[GenerateGiftCode] = { val gen = new SecureRandom() val ints = Iterator.continually(gen.nextInt()) - fromRandom(ints) + fromInts(ints) } - def fromRandom(random: Iterator[Int]): Iterator[GenerateGiftCode] = + def fromInts(random: Iterator[Int]): Iterator[GenerateGiftCode] = CodeSuffixGenerator.generate(random).map(CodeBuilder.build) } diff --git a/support-services/src/test/scala/com/gu/support/redemption/generator/GiftCodeGeneratorSpec.scala b/support-services/src/test/scala/com/gu/support/redemption/generator/GiftCodeGeneratorSpec.scala index 8acc8879dc..348a3ae9c0 100644 --- a/support-services/src/test/scala/com/gu/support/redemption/generator/GiftCodeGeneratorSpec.scala +++ b/support-services/src/test/scala/com/gu/support/redemption/generator/GiftCodeGeneratorSpec.scala @@ -6,7 +6,7 @@ import org.scalatest.matchers.should.Matchers class GiftCodeGeneratorSpec extends AnyFlatSpec with Matchers { it should "work in the basic case" in { - val giftCode = GiftCodeGenerator.fromRandom(Iterator.continually(0)).next.withDuration(GiftDuration.Gift3Month) + val giftCode = GiftCodeGenerator.fromInts(Iterator.continually(0)).next.withDuration(GiftDuration.Gift3Month) giftCode.value should be("gd03-000000") }