testmycode / tmc-core

The common clientside implementation for tmc-clients. Used by the tmc-netbeans and the tmc-cli
3 stars 18 forks source link

Get new exercises in GetUpdatableExercises #104

Closed Salmela closed 8 years ago

Salmela commented 8 years ago

I am still testing these changes so please don't merge it.

Salmela commented 8 years ago

I verified that these changes work on actual course.

jamo commented 8 years ago

Great work! However, I have a few thoughts and feature requests on this implementation.

  1. I would prefer to categorize the updates and new exercises, so that when we ask for users which exercises she wants to download update it's clear which exercises are new and which updates. So maybe instead return a UpdateResult (can be inner class) that contains ImmutableList<Exercise> for new exercises and updates.
  2. I dislike the O(n^n) implementation, with a lookup table we could make this O(n). This might be helpful: https://github.com/testmycode/tmc-core/blob/master/src/main/java/fi/helsinki/cs/tmc/core/domain/ExerciseKey.java
Salmela commented 8 years ago

Please don't merge this pull request yet. I found typo in one of the unit tests.

Salmela commented 8 years ago

This pull request can be now merged. I got the problem fixed.

jamo commented 8 years ago

Awoseme. Nice work!