Thread history

Something else I'm working on is providing a way to interrupt RoboRunner in the middle of a challenge. I'm not sure in what ways that could potentially mess up RoboRunner yet, but I did have to make a couple of changes to make this work:

First, in order to stop RoboRunner completely (and not just the current battle), I had to make an InterruptedException result in the bypass of all queued battles:

In BattleRunner:

  private void getAllFutures(List<Future<String>> futures) {
    for (Future<String> future : futures) {
      try {
        future.get();
      } catch (InterruptedException e) {
        e.printStackTrace();
        return;
      } catch (ExecutionException e) {
        e.printStackTrace();
      }
    }
  }

There might be some additional modification, but for now, I just added a return statement if an InterruptedException occurs (will probably also get rid of the printStackTrace call). This prevents calling get() on all remaining Futures. While I think this was an unexpected condition in RoboRunner, in RoboJogger an InterruptedException is now an expected result whenever a stop command is issued for RoboRunner. A remaining question is, what, if anything, will be broken as a result of this?

Another change I made to ScoreLog, such that trying to access battle results for a "botList" that does not exist will not cause a NullPointerException:

In ScoreLog:

  public List<BattleScore> getBattleScores(String botList) {
    List<BattleScore> scores = _scores.get(botList);
    return (scores == null)? null : ImmutableList.copyOf(_scores.get(botList));
  }

I provided the null check on scores. I was kind of surprised that ImmutableList didn't do that by design. The most likely scenario where this happens is related to my other change -- if a challenge is interrupted before battles have been run against all opponents, when I later access results from the ScoreLog, I am not aware of missing results until the getBattleScores method returns null. I suppose I could have also just added a try/catch in my own code for NullPointerException without having to change RoboRunner, but I felt doing so was not the better way of handling it.

These changes are not finalized. I'm just writing about them for the sake of discussion.

I just noticed that for getting battle results, there is a method hasBotList(String) method that I could call before trying to get battle scores. This would prevent the NPE without modifying RoboRunner. Given this, I could see arguing either way about whether getBattleScores should throw NPE or return null for a botList that does not exist.

I don't have a strong opinion about NPE vs returning null - I think the hasBotList is what made me feel ok with leaving the other one NPE-ing, but that doesn't mean it has to. Seems silly to insist you call 2 methods instead of 1.

I'll have to think about the interruption stuff. Are you using the same RoboRunner instance after interrupting and trying to use it again? Certainly that would give me pause and I'd want to look over RoboRunner and BattleRunner to see what internal state might be confused by this. If not, my only worry would be if the interruption came during a file write to the score log. Maybe in the file writing, we need to catch InterruptedException, close the file stream in the catch, and rethrow? I'm not really sure. Maybe Java is already smart enough not to corrupt a file stream when being interrupted? The code you have here makes sense and doesn't raise any red flags besides that.

No -- I create a new RoboRunner instance for each challenge started. If RoboRunner is interrupted in the middle of a challenge, when RoboRunner is restarted, a new RoboRunner instance is created. Good point on the potential for RoboRunner to be writing to the score log when interrupted; I do need to take a closer look at that.

Instead of dealing with interrupted exceptions, robo runner could just provide a cancel flag that gets checked before each get().