add null checks to all methods and missing unit tests for exceptions already thrown

README.md

@@ -34,3 +34,23 @@ Mexico 0 - Canada 5
 Argentina 3 - Australia 1
 Germany 2 - France 2
 ```
+
+###Some notes on edge cases
+In an exercise where simplicity is requested and edge cases as well, it's virtually impossible to match expectations. This makes this exercise great.
+
+My approach for not dealing with nulls, but still dealing with nulls, in a simple project like this, was this:
+* There is no reference to nulls in the method javadocs, so they are not expected and therefore the behaviour is undetermined
+* In an application using this library, as it was before the commit where these notes got added, the cases where nulls would be passed are extreme and arguably not something that should be dealt with at this level in an app
+* Although I didn't add a test for it, I made sure that an NPE would be the exception being thrown
+* Adding it properly to where it's needed is time-consuming. I have limited time to do these challenges so the compromise was to make sure the correct exceptions were thrown with the code I delivered
+
+I tend to think and code for edge cases that stem from user error, not programmer error. NPEs, as any Runtime Exception, are to be avoided and functionality made to fail gracefully. They most often result from programmer error or hardware faults so... got to pick battles..
+
+####An edge case I got wrong
+Trying to finish a match that is no longer held by the library, or never was, seems like something a user would do by mistake and is definitely something that one such library should trigger an exception for, in general. 
+However, I did everything but add the javadoc throws tag for it. :) Added it now.
+
+Trying to start a match with teams that are already in a game is another example. Or the same team as away and home.
+However, in these I added everything but the unit tests for them.
+
+Adding isInOngoingMatch(ITeam) and isOngoingMatch(IMatch) would also be nice helper methods to have. But..simple library.

src/main/java/org/example/scoreboard/api/IScoreboard.java

@@ -2,6 +2,7 @@ package org.example.scoreboard.api;
 
 import java.security.InvalidParameterException;
 import java.util.ArrayList;
+import java.util.NoSuchElementException;
 
 public interface IScoreboard {
 
@@ -11,7 +12,8 @@ public interface IScoreboard {
    *
    * @param homeTeam the home team
    * @param awayTeam the away team
-   * @throws InvalidParameterException the team is already in a match
+   * @throws InvalidParameterException team is already in a match or the same team is passed as home and away
+   * @throws NullPointerException one, or both, of the passed teams is null
    * @return the match started
    */
   IMatch startMatch(ITeam homeTeam, ITeam awayTeam);
@@ -21,6 +23,8 @@ public interface IScoreboard {
    * It will remove a match from the scoreboard.
    *
    * @param match the match to finish
+   * @throws NoSuchElementException match is not ongoing
+   * @throws NullPointerException passed match is null
    * @return the finished match
    */
   IMatch finishMatch(IMatch match);
@@ -32,6 +36,7 @@ public interface IScoreboard {
    * @param match match to update
    * @param homeTeamScore home team score
    * @param awayTeamScore away team score
+   * @throws NullPointerException passed match is null or sores are null or goals in scores are null
    * @return updated match
    */
   IMatch updateScore(IMatch match, IScore homeTeamScore, IScore awayTeamScore);

src/main/java/org/example/scoreboard/generic/AbstractScoreboard.java

@@ -29,6 +29,10 @@ public class AbstractScoreboard implements IScoreboard {
 
   @Override
   public IMatch startMatch(ITeam homeTeam, ITeam awayTeam) {
+    if (homeTeam == null || awayTeam == null) {
+      throw new NullPointerException();
+    }
+
     if(teamsOnTheBoard.putIfAbsent(homeTeam.getId(), homeTeam) != null) {
       throw new InvalidParameterException("Home team already on the board");
     }
@@ -45,6 +49,9 @@ public class AbstractScoreboard implements IScoreboard {
 
   @Override
   public IMatch finishMatch(IMatch match) {
+    if (match == null) {
+      throw new NullPointerException();
+    }
     if (!ongoingMatches.containsKey(match.getId())) {
       throw new NoSuchElementException();
     }
@@ -55,6 +62,10 @@ public class AbstractScoreboard implements IScoreboard {
 
   @Override
   public IMatch updateScore(IMatch match, IScore homeTeamScore, IScore awayTeamScore) {
+    if (match == null || homeTeamScore == null || awayTeamScore == null || homeTeamScore.getGoals() == null
+        || awayTeamScore.getGoals() == null) {
+      throw new NullPointerException();
+    }
     if (!ongoingMatches.containsKey(match.getId())) {
       throw new NoSuchElementException();
     }

src/test/java/org/example/scoreboard/reference/TestScoreboard.java

@@ -6,6 +6,7 @@ import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
+import java.security.InvalidParameterException;
 import java.util.*;
 import java.util.stream.Stream;
 
@@ -65,6 +66,19 @@ public class TestScoreboard {
     assertNotNull(scoreboard.startMatch(Mexico, Canada));
   }
 
+  // should throw InvalidParameterException if a match is started with a team already on a match
+  @Test
+  public void shouldThrowInvalidParameterException_whenMatchContainsTeamInOngoingMatches() {
+    scoreboard.startMatch(Mexico, Canada);
+    assertThrows(InvalidParameterException.class, () -> scoreboard.startMatch(Mexico, Canada));
+  }
+
+  // should throw InvalidParameterException if a match is between 1 team, home is the same as away team
+  @Test
+  public void shouldThrowInvalidParameterException_whenMatchHasHomeAndAwayAsSameTeam() {
+    assertThrows(InvalidParameterException.class, () -> scoreboard.startMatch(Mexico, Mexico));
+  }
+
   // should finish a game when the game has been started before
   @Test
   public void shouldFinishAGame_whenGameHasBeenStarted() {
@@ -147,4 +161,42 @@ public class TestScoreboard {
     Object[] gameSummary = scoreboard.getGameSummary().toArray();
     assertArrayEquals(expectedResult, gameSummary);
   }
+
+  // should throw NullPointerException when teams are null in start of a match
+  @Test
+  public void shouldThrowNullPointerException_whenTeamsAreNullInStartMatch() {
+    assertThrows(NullPointerException.class, () -> scoreboard.startMatch(null, Canada));
+    assertThrows(NullPointerException.class, () -> scoreboard.startMatch(Mexico, null));
+    assertThrows(NullPointerException.class, () -> scoreboard.startMatch(null, null));
+  }
+
+  // should throw NullPointerException when the team is null in the match finish
+  @Test
+  public void shouldThrowNullPointerException_whenMatchIsNullInFinishMatch() {
+    assertThrows(NullPointerException.class, () -> scoreboard.finishMatch(null));
+  }
+
+  // should throw NullPointerException when the match is null in the match update
+  @Test
+  public void shouldThrowNullPointerException_whenMatchIsNullInUpdateMatch() {
+    assertThrows(NullPointerException.class, () -> scoreboard.updateScore(null, new Score(0), new Score(5)));
+  }
+
+  // should throw NullPointerException when the scores are null in the match update
+  @Test
+  public void shouldThrowNullPointerException_whenScoresAreNullInUpdateMatch() {
+    IMatch match = scoreboard.startMatch(Mexico, Canada);
+    assertThrows(NullPointerException.class, () -> scoreboard.updateScore(match, null, null));
+    assertThrows(NullPointerException.class, () -> scoreboard.updateScore(match, null, new Score(5)));
+    assertThrows(NullPointerException.class, () -> scoreboard.updateScore(match, new Score(0), null));
+  }
+
+  // should throw NullPointerException when the goals in scores are null in the match update
+  @Test
+  public void shouldThrowNullPointerException_whenGoalsInScoresAreNullInUpdateMatch() {
+    IMatch match = scoreboard.startMatch(Mexico, Canada);
+    assertThrows(NullPointerException.class, () -> scoreboard.updateScore(match, new Score(null), new Score(5)));
+    assertThrows(NullPointerException.class, () -> scoreboard.updateScore(match, new Score(0), new Score(null)));
+    assertThrows(NullPointerException.class, () -> scoreboard.updateScore(match, new Score(null), new Score(null)));
+  }
 }