Fixes and cleanup, ready to merge into master.

todolist/core/src/main/java/todolist/core/TodoModel.java

@@ -125,6 +125,6 @@ public class TodoModel implements Iterable<AbstractTodoList> {
    * @return a function that gets the todo items of a todo list in the corresponding sort order
    */
   public Function<TodoList, Collection<TodoItem>> getSortedTodoItemsProvider() {
-    return getSortedTodoItemsProvider(getSettings().getTodoItemSortOrder());
+    return getSortedTodoItemsProvider(getSettings().getTodoItemsSortOrder());
   }
 }

todolist/core/src/main/java/todolist/core/TodoSettings.java

 package todolist.core;
 
 import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -9,6 +8,11 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
 
+/**
+ * A class for managing settings,
+ * i.e. properties that control behavior of other classes.
+ * Changes to settings can be listened to and vetoed by throwing an exception.
+ */
 public class TodoSettings {
 
   private Collection<TodoSettingsListener> todoSettingsListeners = new ArrayList<>();
@@ -23,7 +27,7 @@ public class TodoSettings {
 
   private Map<String, Object> oldValues = null;
 
-  private void fireSettingChanged(String property, Object oldValue, Object newValue) {
+  private void handleSettingChanged(String property, Object oldValue, Object newValue) {
     if (Objects.equals(oldValue, newValue)) {
       return;
     }
@@ -48,21 +52,28 @@ public class TodoSettings {
     }
   }
 
+  /**
+   * Call to indicate that new values will be used, and
+   * old values can be forgotten.
+   */
   public void applyChanges() {
     oldValues = null;
   }
 
   private void cancelChange(String property) {
-    Object oldValue = oldValues.get(property);
+    var oldValue = oldValues.get(property);
+    var setterName = "set" + Character.toUpperCase(property.charAt(0)) + property.substring(1);
     try {
-      Method setter = getClass().getMethod("set" + Character.toUpperCase(property.charAt(0)) + property.substring(1),
-          oldValue.getClass());
+      var setter = getClass().getMethod(setterName, oldValue.getClass());
       setter.invoke(this, oldValue);
     } catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e) {
       // ignore
     }
   }
 
+  /**
+   * Call to rollback changes to old values.
+   */
   public void cancelChanges() {
     try {
       for (String property : oldValues.keySet()) {
@@ -75,21 +86,30 @@ public class TodoSettings {
 
   //
 
+  /**
+   * Enum for possible values of sort order for items.
+   */
   public enum TodoItemsSortOrder {
     NONE, UNCHECKED_CHECKED, CHECKED_UNCHECKED
   }
 
-  private TodoItemsSortOrder todoItemSortOrder = TodoItemsSortOrder.UNCHECKED_CHECKED;
+  private TodoItemsSortOrder todoItemsSortOrder = TodoItemsSortOrder.NONE;
 
-  public TodoItemsSortOrder getTodoItemSortOrder() {
-    return todoItemSortOrder;
+  public TodoItemsSortOrder getTodoItemsSortOrder() {
+    return todoItemsSortOrder;
   }
 
-  public final static String TODO_ITEM_SORT_ORDER_SETTING = "todoItemSortOrder";
-
-  public void setTodoItemSortOrder(TodoItemsSortOrder todoItemSortOrder) {
-    Object oldValue = this.todoItemSortOrder;
-    this.todoItemSortOrder = todoItemSortOrder;
-    fireSettingChanged(TODO_ITEM_SORT_ORDER_SETTING, oldValue, todoItemSortOrder);
+  // settings name must correspond to setter name!
+  public static final String TODO_ITEM_SORT_ORDER_SETTING = "todoItemsSortOrder";
+
+  /**
+   * Sets the todoItemsSortOrder property, and notifies listeners.
+   *
+   * @param todoItemSortOrder the new todoItemSortOrder value
+   */
+  public void setTodoItemsSortOrder(TodoItemsSortOrder todoItemSortOrder) {
+    Object oldValue = this.todoItemsSortOrder;
+    this.todoItemsSortOrder = todoItemSortOrder;
+    handleSettingChanged(TODO_ITEM_SORT_ORDER_SETTING, oldValue, todoItemSortOrder);
   }
 }

todolist/core/src/main/java/todolist/core/TodoSettingsListener.java

@@ -2,6 +2,17 @@ package todolist.core;
 
 import java.util.Collection;
 
+/**
+ * Listener for changes to TodoSettings.
+ */
 public interface TodoSettingsListener {
+
+  /**
+   * Called when settings are changed.
+   * Changes may be vetoed by throwing an exception.
+   *
+   * @param settings the TodoSettings that changed
+   * @param changedProperties the properties (names) that changed 
+   */
   public void todoSettingsChanged(TodoSettings settings, Collection<String> changedProperties);
 }
\ No newline at end of file

todolist/core/src/main/java/todolist/json/internal/TodoSettingsDeserializer.java

@@ -29,7 +29,7 @@ class TodoSettingsDeserializer extends JsonDeserializer<TodoSettings> {
       if (todoItemsSortOrderNode instanceof TextNode) {
         try {
           TodoItemsSortOrder sortOrder = TodoItemsSortOrder.valueOf(todoItemsSortOrderNode.asText());
-          settings.setTodoItemSortOrder(sortOrder);
+          settings.setTodoItemsSortOrder(sortOrder);
         } catch (IllegalArgumentException iae) {
           // ignore unknown sort order constant
         }

todolist/core/src/main/java/todolist/json/internal/TodoSettingsSerializer.java

@@ -9,7 +9,7 @@ import todolist.core.TodoSettings;
 class TodoSettingsSerializer extends JsonSerializer<TodoSettings> {
 
   /*
-   * format: { "todoItemSortOrder": "..." }
+   * format: { "todoItemsSortOrder": "..." }
    */
 
   @Override
@@ -17,7 +17,7 @@ class TodoSettingsSerializer extends JsonSerializer<TodoSettings> {
       SerializerProvider serializerProvider)
       throws IOException {
     jsonGen.writeStartObject();
-    jsonGen.writeStringField(TodoSettings.TODO_ITEM_SORT_ORDER_SETTING, settings.getTodoItemSortOrder().name());
+    jsonGen.writeStringField(TodoSettings.TODO_ITEM_SORT_ORDER_SETTING, settings.getTodoItemsSortOrder().name());
     jsonGen.writeEndObject();
   }
 }

todolist/core/src/test/java/todolist/core/TodoModelTest.java

@@ -78,7 +78,7 @@ public class TodoModelTest {
     List<TodoItem> allAddedItems = new ArrayList<>(todoList.getTodoItems());
     Collection<TodoItem> expectedOrder = Stream.of(sortedTodoItemIndices).map(allAddedItems::get).collect(Collectors.toList());
     TodoListTest.checkItems(TodoModel.getSortedTodoItemsProvider(sortOrder).apply(todoList), expectedOrder.toArray(new TodoItem[expectedOrder.size()]));
-    todoModel.getSettings().setTodoItemSortOrder(sortOrder);
+    todoModel.getSettings().setTodoItemsSortOrder(sortOrder);
     TodoListTest.checkItems(todoModel.getSortedTodoItemsProvider().apply(todoList), expectedOrder.toArray(new TodoItem[expectedOrder.size()]));
   }
 

todolist/core/src/test/java/todolist/core/TodoSettingsTest.java

@@ -19,32 +19,32 @@ public class TodoSettingsTest {
   @BeforeEach
   public void setup() {
     todoSettings = new TodoSettings();
-    todoSettings.setTodoItemSortOrder(TodoItemsSortOrder.NONE);
+    todoSettings.setTodoItemsSortOrder(TodoItemsSortOrder.NONE);
     todoSettings.applyChanges();
   }
 
   @Test
   public void testSetTodoItemsSortOrder_changesProperty() {
-    todoSettings.setTodoItemSortOrder(TodoItemsSortOrder.CHECKED_UNCHECKED);
-    assertEquals(TodoItemsSortOrder.CHECKED_UNCHECKED, todoSettings.getTodoItemSortOrder());
+    todoSettings.setTodoItemsSortOrder(TodoItemsSortOrder.CHECKED_UNCHECKED);
+    assertEquals(TodoItemsSortOrder.CHECKED_UNCHECKED, todoSettings.getTodoItemsSortOrder());
   }
 
-  private static Collection<String> TODO_ITEMS_SORT_ORDER_SINGLETON_COLLECTION = Collections.singleton("todoItemSortOrder");
+  private static Collection<String> TODO_ITEMS_SORT_ORDER_SINGLETON_COLLECTION = Collections.singleton(TodoSettings.TODO_ITEM_SORT_ORDER_SETTING);
 
   @Test
   public void testSetTodoItemsSortOrder_todoSettingsChangedCalledWhenPropertyChanged() {
     TodoSettingsListener todoSettingsListener = mock(TodoSettingsListener.class);
     todoSettings.addTodoSettingsListener(todoSettingsListener);
-    todoSettings.setTodoItemSortOrder(TodoItemsSortOrder.CHECKED_UNCHECKED);
-    assertEquals(TodoItemsSortOrder.CHECKED_UNCHECKED, todoSettings.getTodoItemSortOrder());
+    todoSettings.setTodoItemsSortOrder(TodoItemsSortOrder.CHECKED_UNCHECKED);
+    assertEquals(TodoItemsSortOrder.CHECKED_UNCHECKED, todoSettings.getTodoItemsSortOrder());
     // listener called when property changed
     verify(todoSettingsListener, times(1)).todoSettingsChanged(todoSettings, TODO_ITEMS_SORT_ORDER_SINGLETON_COLLECTION);
-    todoSettings.setTodoItemSortOrder(TodoItemsSortOrder.CHECKED_UNCHECKED);
-    assertEquals(TodoItemsSortOrder.CHECKED_UNCHECKED, todoSettings.getTodoItemSortOrder());
+    todoSettings.setTodoItemsSortOrder(TodoItemsSortOrder.CHECKED_UNCHECKED);
+    assertEquals(TodoItemsSortOrder.CHECKED_UNCHECKED, todoSettings.getTodoItemsSortOrder());
     // listener not called when property didn't change
     verify(todoSettingsListener, times(1)).todoSettingsChanged(todoSettings, TODO_ITEMS_SORT_ORDER_SINGLETON_COLLECTION);
-    todoSettings.setTodoItemSortOrder(TodoItemsSortOrder.UNCHECKED_CHECKED);
-    assertEquals(TodoItemsSortOrder.UNCHECKED_CHECKED, todoSettings.getTodoItemSortOrder());
+    todoSettings.setTodoItemsSortOrder(TodoItemsSortOrder.UNCHECKED_CHECKED);
+    assertEquals(TodoItemsSortOrder.UNCHECKED_CHECKED, todoSettings.getTodoItemsSortOrder());
     // listener called another time when property changed
     verify(todoSettingsListener, times(2)).todoSettingsChanged(todoSettings, TODO_ITEMS_SORT_ORDER_SINGLETON_COLLECTION);
   }
@@ -53,7 +53,7 @@ public class TodoSettingsTest {
   public void testSetTodoItemsSortOrder_todoSettingsChangedNotCalledWhenPropertyNotChanged() {
     TodoSettingsListener todoSettingsListener = mock(TodoSettingsListener.class);
     todoSettings.addTodoSettingsListener(todoSettingsListener);
-    todoSettings.setTodoItemSortOrder(TodoItemsSortOrder.NONE);
+    todoSettings.setTodoItemsSortOrder(TodoItemsSortOrder.NONE);
     verifyNoInteractions(todoSettingsListener);
   }
 
@@ -61,22 +61,22 @@ public class TodoSettingsTest {
   public void testSetTodoItemsSortOrder_applyChanges() {
     TodoSettingsListener todoSettingsListener = mock(TodoSettingsListener.class);
     todoSettings.addTodoSettingsListener(todoSettingsListener);
-    todoSettings.setTodoItemSortOrder(TodoItemsSortOrder.CHECKED_UNCHECKED);
+    todoSettings.setTodoItemsSortOrder(TodoItemsSortOrder.CHECKED_UNCHECKED);
     // listener called when property changed
     verify(todoSettingsListener, times(1)).todoSettingsChanged(todoSettings, TODO_ITEMS_SORT_ORDER_SINGLETON_COLLECTION);
     todoSettings.applyChanges();
     // apply changes not canceled
-    assertEquals(TodoItemsSortOrder.CHECKED_UNCHECKED, todoSettings.getTodoItemSortOrder());
+    assertEquals(TodoItemsSortOrder.CHECKED_UNCHECKED, todoSettings.getTodoItemsSortOrder());
   }
 
   @Test
   public void testSetTodoItemsSortOrder_cancelChanges() {
     TodoSettingsListener todoSettingsListener = mock(TodoSettingsListener.class);
     todoSettings.addTodoSettingsListener(todoSettingsListener);
-    todoSettings.setTodoItemSortOrder(TodoItemsSortOrder.CHECKED_UNCHECKED);
+    todoSettings.setTodoItemsSortOrder(TodoItemsSortOrder.CHECKED_UNCHECKED);
     todoSettings.cancelChanges();
     // property change(s) rolled back
-    assertEquals(TodoItemsSortOrder.NONE, todoSettings.getTodoItemSortOrder());
+    assertEquals(TodoItemsSortOrder.NONE, todoSettings.getTodoItemsSortOrder());
   }
 
   @Test
@@ -84,9 +84,9 @@ public class TodoSettingsTest {
     TodoSettingsListener todoSettingsListener = mock(TodoSettingsListener.class);
     doThrow(IllegalStateException.class).when(todoSettingsListener).todoSettingsChanged(todoSettings, TODO_ITEMS_SORT_ORDER_SINGLETON_COLLECTION);
     todoSettings.addTodoSettingsListener(todoSettingsListener);
-    todoSettings.setTodoItemSortOrder(TodoItemsSortOrder.CHECKED_UNCHECKED);
+    todoSettings.setTodoItemsSortOrder(TodoItemsSortOrder.CHECKED_UNCHECKED);
     // property change rolled back
-    assertEquals(TodoItemsSortOrder.NONE, todoSettings.getTodoItemSortOrder());
+    assertEquals(TodoItemsSortOrder.NONE, todoSettings.getTodoItemsSortOrder());
     verify(todoSettingsListener, times(2)).todoSettingsChanged(todoSettings, TODO_ITEMS_SORT_ORDER_SINGLETON_COLLECTION);
   }
 }

todolist/core/src/test/java/todolist/json/TodoModuleTest.java

@@ -132,12 +132,12 @@ public class TodoModuleTest {
   @Test
   public void testTodoSettings() {
     TodoSettings settings = new TodoSettings();
-    settings.setTodoItemSortOrder(TodoItemsSortOrder.UNCHECKED_CHECKED);
+    settings.setTodoItemsSortOrder(TodoItemsSortOrder.UNCHECKED_CHECKED);
     try {
       String json = mapper.writeValueAsString(settings);
       assertEquals(defaultTodoSettings.replaceAll("\\s+", ""), mapper.writeValueAsString(settings));
       TodoSettings settings2 = mapper.readValue(json, TodoSettings.class);
-      assertEquals(settings.getTodoItemSortOrder(), settings2.getTodoItemSortOrder());
+      assertEquals(settings.getTodoItemsSortOrder(), settings2.getTodoItemsSortOrder());
     } catch (JsonProcessingException e) {
       fail(e.getMessage());
     }

todolist/fxui/src/main/java/todolist/ui/TodoItemListCellDragHandler.java

@@ -8,6 +8,9 @@ import javafx.scene.input.TransferMode;
 import todolist.core.TodoItem;
 import todolist.core.TodoList;
 
+/**
+ * Handles dragging of ListCells in a ListView of TodoItems in a TodoList.
+ */
 public class TodoItemListCellDragHandler {
 
   private TodoList todoList;

todolist/fxui/src/main/java/todolist/ui/TodoModelController.java

@@ -15,8 +15,8 @@ import todolist.core.AbstractTodoList;
 import todolist.core.TodoList;
 import todolist.core.TodoModel;
 import todolist.core.TodoSettings;
-import todolist.core.TodoSettingsListener;
 import todolist.core.TodoSettings.TodoItemsSortOrder;
+import todolist.core.TodoSettingsListener;
 import todolist.ui.util.SceneTarget;
 
 /**
@@ -40,6 +40,12 @@ public class TodoModelController implements TodoSettingsListener {
   @FXML
   TodoListController todoListViewController;
 
+  /**
+   * Sets the TodoModelAccess for this controller,
+   * so data can come from different sources.
+   *
+   * @param todoModelAccess the new TodoModelAccess to use
+   */
   public void setTodoModelAccess(TodoModelAccess todoModelAccess) {
     this.todoModelAccess = todoModelAccess;
     updateTodoItemsProvider();
@@ -47,6 +53,7 @@ public class TodoModelController implements TodoSettingsListener {
     todoModelAccess.getTodoSettings().addTodoSettingsListener(this);
   }
   
+  @Override
   public void todoSettingsChanged(TodoSettings settings, Collection<String> changedProperties) {
     if (changedProperties.contains(TodoSettings.TODO_ITEM_SORT_ORDER_SETTING)) {
       updateTodoItemsProvider();
@@ -54,7 +61,7 @@ public class TodoModelController implements TodoSettingsListener {
   }
 
   private void updateTodoItemsProvider() {
-    TodoItemsSortOrder sortOrder = this.todoModelAccess.getTodoSettings().getTodoItemSortOrder();
+    TodoItemsSortOrder sortOrder = this.todoModelAccess.getTodoSettings().getTodoItemsSortOrder();
     todoListViewController.setTodoItemsProvider(TodoModel.getSortedTodoItemsProvider(sortOrder));
   }
 

todolist/fxui/src/main/java/todolist/ui/TodoSettingsController.java

@@ -7,6 +7,9 @@ import todolist.core.TodoSettings;
 import todolist.core.TodoSettings.TodoItemsSortOrder;
 import todolist.ui.util.SceneTarget;
 
+/**
+ * Controller for editing TodoSettings.
+ */
 public class TodoSettingsController {
   
   private TodoSettings todoSettings = new TodoSettings();
@@ -30,7 +33,7 @@ public class TodoSettingsController {
   void initialize() {
     todoItemsSortOrderSelector.setOnAction(actionEvent -> {
       int ordinal = todoItemsSortOrderSelector.getSelectionModel().getSelectedIndex();
-      getTodoSettings().setTodoItemSortOrder(TodoItemsSortOrder.values()[ordinal]);
+      getTodoSettings().setTodoItemsSortOrder(TodoItemsSortOrder.values()[ordinal]);
     });
     updateView();
   }
@@ -40,7 +43,7 @@ public class TodoSettingsController {
   }
 
   private void updateView() {
-    TodoItemsSortOrder sortOrder = getTodoSettings().getTodoItemSortOrder();
+    TodoItemsSortOrder sortOrder = getTodoSettings().getTodoItemsSortOrder();
     todoItemsSortOrderSelector.getSelectionModel().select(sortOrder.ordinal());
   }
 }
\ No newline at end of file

todolist/fxui/src/main/java/todolist/ui/util/SceneTarget.java

@@ -6,6 +6,9 @@ import javafx.scene.Scene;
 import javafx.scene.control.Control;
 import javafx.stage.Stage;
 
+/**
+ * Helper class for buttons that transition to a specific scene.
+ */
 public class SceneTarget {
 
   private Scene scene;

todolist/rest/src/main/java/todolist/restapi/TodoSettingsResource.java

@@ -44,8 +44,7 @@ public class TodoSettingsResource {
   /**
    * Replaces the TodoSettings.
    *
-   * @param todoSettings the todoSettings to set
-   * @return true if it was added, false if it replaced
+   * @param todoSettings the new TodoSettings to use
    */
   @PUT
   @Consumes(MediaType.APPLICATION_JSON)