Zoned date time

README.md

@@ -16,7 +16,7 @@ as well as the logic to recommend the cheapest price plan for a particular house
 
 >Unfortunately, as the codebase has evolved, it has gathered tech debt in the form of a number of code smells and some 
 questionable design decisions. Our goal for the upcoming exercise would be to deliver value by implementing a new 
-feature using _Test Driven Development_ (TDD), while refactoring away the code smells we see. 
+feature using _[Test Driven Development](https://martinfowler.com/bliki/TestDrivenDevelopment.html)_ (TDD), while refactoring away the code smells we see. 
 > 
 >In preparation for this, please take some time to go through the code and identify any improvements, big or small, 
 that would improve its maintainability, testability, and design.
@@ -46,7 +46,9 @@ The project requires [Java 21](https://adoptium.net/) or higher.
 
 ## Useful commands
 
-Compile the project, run the tests and creates an executable JAR file
+### Build the project
+
+Compile the project, run the tests and creates an executable JAR file:
 
 ```console
 ./gradlew build
@@ -67,11 +69,10 @@ You can run it with the following command:
 
 ## API Documentation
 
-The codebase contains two service classes, _MeterReadingManager_ and _PricePlanComparator_, that serve as entry points to
-the implemented features.
+The codebase contains two service classes, `MeterReadingManager` and `PricePlanComparator` that serve as entry points to the implemented features.
 
 ### MeterReadingManager
-Provides methods to store and fetch the energy consumption readings from a given Smart Meter
+Provides methods to store and fetch the energy consumption readings from a given Smart Meter.
 
 > #### _public void_ storeReadings(_String smartMeterId, List<ElectricityReading> electricityReadings_)
 Stores the provided _ElectricityReading_ collection in the indicated _SmartMeter_. If no 
@@ -91,13 +92,13 @@ An _ElectricityReading_ record consists of the following fields:
 
 Example readings
 
-| Date (`GMT`)      | Epoch timestamp | Reading (kWh) |
-|-------------------|----------------:|--------------:|
-| `2020-11-29 8:00` |      1606636800 |        600.05 |
-| `2020-11-29 9:00` |      1606640400 |        602.06 |
-| `2020-11-30 7:30` |      1606721400 |        610.09 |
-| `2020-12-01 8:30` |      1606811400 |        627.12 |
-| `2020-12-02 8:30` |      1606897800 |        635.14 |
+| Date (`GMT`)      | Epoch timestamp (seconds) | Reading (kWh) |
+|-------------------|--------------------------:|--------------:|
+| `2020-11-29 8:00` |                1606636800 |        600.05 |
+| `2020-11-29 9:00` |                1606640400 |        602.06 |
+| `2020-11-30 7:30` |                1606721400 |        610.09 |
+| `2020-12-01 8:30` |                1606811400 |        627.12 |
+| `2020-12-02 8:30` |                1606897800 |        635.14 |
 
 Thee above table shows some readings sampled by a smart meter over multiple days. Note that since the smart 
 meter is reporting the total energy consumed up to that point in time, a reading's value will always be higher or the same as 

src/main/java/tw/joi/energy/config/TestData.java

@@ -14,7 +14,7 @@ public final class TestData {
     private static final PricePlan MOST_EVIL_PRICE_PLAN =
             new PricePlan("price-plan-0", "Dr Evil's Dark Energy", BigDecimal.TEN, emptyList());
     private static final PricePlan RENEWABLES_PRICE_PLAN =
-            new PricePlan("price-plan-1", "The Green Eco", BigDecimal.valueOf(2), null);
+            new PricePlan("price-plan-1", "The Green Eco", BigDecimal.valueOf(2), emptyList());
     private static final PricePlan STANDARD_PRICE_PLAN =
             new PricePlan("price-plan-2", "Power for Everyone", BigDecimal.ONE, emptyList());
 

src/main/java/tw/joi/energy/domain/ElectricityReading.java

@@ -4,6 +4,7 @@
 import java.time.Instant;
 
 /**
- * @param reading kWh
+ * @param time point in time
+ * @param readingInKwH energy consumed in total to this point in time in kWh
  */
-public record ElectricityReading(Instant time, BigDecimal reading) {}
+public record ElectricityReading(Instant time, BigDecimal readingInKwH) {}

src/main/java/tw/joi/energy/domain/PricePlan.java

@@ -1,21 +1,20 @@
 package tw.joi.energy.domain;
 
-import java.io.*;
 import java.math.BigDecimal;
-import java.text.*;
 import java.time.DayOfWeek;
 import java.time.LocalDateTime;
+import java.time.ZonedDateTime;
 import java.util.List;
 
 public class PricePlan {
 
-    private String energySupplier;
-    private String planName;
+    private final String energySupplier;
+    private final String planName;
     private final BigDecimal unitRate; // unit price per kWh
     private final List<PeakTimeMultiplier> peakTimeMultipliers;
 
     public PricePlan(
-            String planName, String energySupplier, BigDecimal unitRate, List<PeakTimeMultiplier> peakTimeMultipliers) {
+        String planName, String energySupplier, BigDecimal unitRate, List<PeakTimeMultiplier> peakTimeMultipliers) {
         this.planName = planName;
         this.energySupplier = energySupplier;
         this.unitRate = unitRate;
@@ -26,23 +25,15 @@ public String getEnergySupplier() {
         return energySupplier;
     }
 
-    public void setEnergySupplier(String supplierName) {
-        this.energySupplier = supplierName;
-    }
-
     public String getPlanName() {
         return planName;
     }
 
-    public void setPlanName(String name) {
-        this.planName = name;
-    }
-
     public BigDecimal getUnitRate() {
         return unitRate;
     }
 
-    public BigDecimal getPrice(LocalDateTime dateTime) {
+    public BigDecimal getPrice(ZonedDateTime dateTime) {
         return unitRate;
     }
 

src/main/java/tw/joi/energy/repository/PricePlanRepository.java

@@ -5,10 +5,10 @@
 
 import java.math.BigDecimal;
 import java.time.LocalDateTime;
+import java.time.ZonedDateTime;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
-import java.util.regex.*;
 import tw.joi.energy.domain.ElectricityReading;
 import tw.joi.energy.domain.PricePlan;
 import tw.joi.energy.domain.SmartMeter;
@@ -34,8 +34,8 @@ private BigDecimal calculateCost(Collection<ElectricityReading> electricityReadi
                 .max(comparing(ElectricityReading::time))
                 .get();
 
-        BigDecimal energyConsumed = latest.reading().subtract(oldest.reading());
-        return energyConsumed.multiply(pricePlan.getPrice(LocalDateTime.now()));
+        BigDecimal energyConsumed = latest.readingInKwH().subtract(oldest.readingInKwH());
+        return energyConsumed.multiply(pricePlan.getPrice(ZonedDateTime.now()));
     }
 
     public List<PricePlan> getAllPricePlans() {

src/test/java/tw/joi/energy/domain/PricePlanTest.java

@@ -1,39 +1,46 @@
 package tw.joi.energy.domain;
 
-import org.assertj.core.data.Percentage;
-import org.junit.jupiter.api.Test;
+import static java.util.Collections.emptyList;
+import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;
 
 import java.math.BigDecimal;
 import java.time.LocalDateTime;
 import java.time.Month;
-
-import static java.util.Collections.emptyList;
-import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat;
+import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.ZonedDateTime;
+import java.util.TimeZone;
+import org.assertj.core.data.Percentage;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
 
 public class PricePlanTest {
 
     @Test
+    @DisplayName("Get energy supplier should return supplier if not null")
     public void get_energy_supplier_should_return_the_energy_supplier_given_supplier_is_existent() {
-        PricePlan pricePlan = new PricePlan(null, "Energy Supplier Name", null, null);
+        PricePlan pricePlan = new PricePlan("Test Plan Name", "Energy Supplier Name", BigDecimal.ONE, emptyList());
 
         assertThat(pricePlan.getEnergySupplier()).isEqualTo("Energy Supplier Name");
     }
 
     @Test
-    public void get_price_should_return_the_base_price_given_an_ordinary_date_time() throws Exception {
-        LocalDateTime normalDateTime = LocalDateTime.of(2017, Month.AUGUST, 31, 12, 0, 0);
-        PricePlan pricePlan = new PricePlan(null, null, BigDecimal.ONE, emptyList());
+    @DisplayName("Get price should return price given non-peak date and time")
+    public void get_price_should_return_the_base_price_given_an_ordinary_date_time() {
+        ZonedDateTime nonPeakDateTime = ZonedDateTime.of(LocalDateTime.of(2017, Month.AUGUST, 31, 12, 0, 0),
+            ZoneId.of("GMT"));
+        // the price plan has no peak days....
+        PricePlan pricePlan = new PricePlan("test plan", "test supplier", BigDecimal.ONE, emptyList());
 
-        BigDecimal price = pricePlan.getPrice(normalDateTime);
+        BigDecimal price = pricePlan.getPrice(nonPeakDateTime);
 
-        assertThat(price).isCloseTo(BigDecimal.ONE, Percentage.withPercentage(1));
+        assertThat(price).isEqualTo(BigDecimal.ONE);
     }
 
     @Test
+    @DisplayName("Get unit rate should return unit rate if no null")
     public void get_unit_rate_should_return_unit_rate_given_unit_rate_is_present() {
-        PricePlan pricePlan = new PricePlan(null, null, BigDecimal.TWO, null);
-        pricePlan.setPlanName("test-price-plan");
-        pricePlan.setEnergySupplier("test-energy-supplier");
+        PricePlan pricePlan = new PricePlan("test-price-plan", "test-energy-supplier", BigDecimal.TWO, emptyList());
 
         BigDecimal rate = pricePlan.getUnitRate();
 

src/test/java/tw/joi/energy/domain/SmartMeterTest.java

@@ -1,14 +1,15 @@
 package tw.joi.energy.domain;
 
-import org.junit.jupiter.api.Test;
+import static org.assertj.core.api.Assertions.assertThat;
 
 import java.util.Collections;
-
-import static org.assertj.core.api.Assertions.assertThat;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
 
 class SmartMeterTest {
 
     @Test
+    @DisplayName("Price plan should be null if none has been supplied")
     void price_plan_id_should_be_null_given_no_price_plan_has_been_provided() {
         var smartMeter = new SmartMeter(null, Collections.emptyList());
 

src/test/java/tw/joi/energy/repository/PricePlanRepositoryTest.java

@@ -3,11 +3,13 @@
 import static java.util.Collections.emptyList;
 import static org.assertj.core.api.Assertions.assertThat;
 
+import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 
 class PricePlanRepositoryTest {
 
     @Test
+    @DisplayName("Should return empty list of plans if none available")
     void should_return_empty_list_when_get_all_price_plans_given_no_price_plans_available() {
         var repository = new PricePlanRepository(emptyList());
 

src/test/java/tw/joi/energy/repository/SmartMeterRepositoryTest.java

@@ -3,19 +3,22 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 import java.util.List;
+import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
 import tw.joi.energy.domain.SmartMeter;
 
 class SmartMeterRepositoryTest {
 
     @Test
+    @DisplayName("findById should return empty option when searching for non-existent id")
     void should_return_empty_smart_meter_when_find_by_id_given_a_non_existent_id() {
         var repository = new SmartMeterRepository();
 
         assertThat(repository.findById("non-existent")).isEmpty();
     }
 
     @Test
+    @DisplayName("findById should return appropriate smart meter if parameter exists in repository")
     void should_return_smart_meters_when_find_by_id_given_existent_smart_meter_ids() {
         var repository = new SmartMeterRepository();
         SmartMeter smartMeter0 = new SmartMeter(null, List.of());

src/test/java/tw/joi/energy/service/MeterReadingManagerTest.java

@@ -1,54 +1,59 @@
 package tw.joi.energy.service;
 
-import org.junit.jupiter.api.Test;
-import tw.joi.energy.domain.ElectricityReading;
-import tw.joi.energy.repository.SmartMeterRepository;
-
-import java.time.LocalDate;
-import java.util.ArrayList;
-import java.util.List;
-
 import static java.util.Collections.emptyList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static tw.joi.energy.fixture.ElectricityReadingFixture.createReading;
 
+import java.time.LocalDate;
+import java.util.ArrayList;
+import java.util.List;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import tw.joi.energy.domain.ElectricityReading;
+import tw.joi.energy.repository.SmartMeterRepository;
+
 public class MeterReadingManagerTest {
 
     private static final String SMART_METER_ID = "10101010";
     private final SmartMeterRepository smartMeterRepository = new SmartMeterRepository();
     private final MeterReadingManager meterReadingManager = new MeterReadingManager(smartMeterRepository);
 
     @Test
+    @DisplayName("storeReadings should throw exception given a null meterId")
     public void store_readings_should_throw_exception_given_meter_id_is_null() {
         assertThatThrownBy(() -> meterReadingManager.storeReadings(null, emptyList()))
                 .isInstanceOf(IllegalArgumentException.class)
                 .hasMessageContaining("smartMeterId");
     }
 
     @Test
+    @DisplayName("storeReadings should throw exception given meterId is empty string")
     public void store_readings_should_throw_exception_given_meter_id_is_empty() {
         assertThatThrownBy(() -> meterReadingManager.storeReadings("", emptyList()))
                 .isInstanceOf(IllegalArgumentException.class)
                 .hasMessageContaining("smartMeterId");
     }
 
     @Test
+    @DisplayName("storeReadings should throw exception given readings is null")
     public void store_readings_should_throw_exception_given_readings_is_null() {
         assertThatThrownBy(() -> meterReadingManager.storeReadings(SMART_METER_ID, null))
                 .isInstanceOf(IllegalArgumentException.class)
                 .hasMessageContaining("readings");
     }
 
     @Test
+    @DisplayName("storeReadings should throw exception given readings is emtpy list")
     public void store_readings_should_throw_exception_given_readings_is_empty() {
         assertThatThrownBy(() -> meterReadingManager.storeReadings(SMART_METER_ID, emptyList()))
                 .isInstanceOf(IllegalArgumentException.class)
                 .hasMessageContaining("readings");
     }
 
     @Test
-    public void store_readings_should_success_given_meter_readings() {
+    @DisplayName("storeReadings should succeed given non-empty list of readings")
+    public void store_readings_should_succeed_given_meter_readings() {
         var readingsToStore = List.of(createReading(LocalDate.now(), 1.0));
 
         meterReadingManager.storeReadings(SMART_METER_ID, readingsToStore);
@@ -58,7 +63,8 @@ public void store_readings_should_success_given_meter_readings() {
     }
 
     @Test
-    public void store_readings_should_success_given_multiple_batches_of_meter_readings() {
+    @DisplayName("storeReadings should succeed when called multiple times")
+    public void store_readings_should_succeed_given_multiple_batches_of_meter_readings() {
         var meterReadings = List.of(createReading(LocalDate.now(), 1.0));
         var otherMeterReadings = List.of(createReading(LocalDate.now(), 2.0));
 
@@ -74,8 +80,8 @@ public void store_readings_should_success_given_multiple_batches_of_meter_readin
     }
 
     @Test
-    public void
-            readings_should_store_to_associate_meter_given_multiple_meters_are_existent() {
+    @DisplayName("storeReadings should write supplied readings to correct meter")
+    public void store_readings_should_store_to_correct_meter_given_multiple_meters_exist() {
         var meterReadings = List.of(createReading(LocalDate.now(), 1.0));
         var otherMeterReadings = List.of(createReading(LocalDate.now(), 2.0));
 
@@ -87,13 +93,15 @@ public void store_readings_should_success_given_multiple_batches_of_meter_readin
     }
 
     @Test
-    public void read_readings_should_throw_exception_given_meter_id_is_not_existent() {
+    @DisplayName("readReadings should throw exception if supplied meterId is not persisted")
+    public void read_readings_should_throw_exception_given_meter_id_is_not_persisted() {
         assertThatThrownBy(() -> meterReadingManager.readReadings(SMART_METER_ID))
                 .isInstanceOf(IllegalArgumentException.class)
                 .hasMessageContaining("smartMeterId");
     }
 
     @Test
+    @DisplayName("readReadings should return previously supplied readings for a known meterId")
     public void read_readings_should_return_readings_given_readings_are_existent() {
         // given
         var meterReadings = List.of(createReading(LocalDate.now(), 1.0));