From 05e03cc75b794bed9231b104b1412dfe64dadaa4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Hallvard=20Tr=C3=A6tteberg?= <hal@ntnu.no>
Date: Wed, 13 Oct 2021 08:54:11 +0000
Subject: [PATCH] Fixes and cleanup, ready to merge into master.
---
.../main/java/todolist/core/TodoModel.java | 2 +-
.../main/java/todolist/core/TodoSettings.java | 48 +++++++++++++------
.../todolist/core/TodoSettingsListener.java | 11 +++++
.../internal/TodoSettingsDeserializer.java | 2 +-
.../json/internal/TodoSettingsSerializer.java | 4 +-
.../java/todolist/core/TodoModelTest.java | 2 +-
.../java/todolist/core/TodoSettingsTest.java | 34 ++++++-------
.../java/todolist/json/TodoModuleTest.java | 4 +-
.../ui/TodoItemListCellDragHandler.java | 3 ++
.../java/todolist/ui/TodoModelController.java | 11 ++++-
.../todolist/ui/TodoSettingsController.java | 7 ++-
.../java/todolist/ui/util/SceneTarget.java | 3 ++
.../restapi/TodoSettingsResource.java | 3 +-
13 files changed, 90 insertions(+), 44 deletions(-)
diff --git a/todolist/core/src/main/java/todolist/core/TodoModel.java b/todolist/core/src/main/java/todolist/core/TodoModel.java
index f95c5c5..03dc70b 100644
--- a/todolist/core/src/main/java/todolist/core/TodoModel.java
+++ b/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());
}
}
diff --git a/todolist/core/src/main/java/todolist/core/TodoSettings.java b/todolist/core/src/main/java/todolist/core/TodoSettings.java
index bac61bc..4c8c751 100644
--- a/todolist/core/src/main/java/todolist/core/TodoSettings.java
+++ b/todolist/core/src/main/java/todolist/core/TodoSettings.java
@@ -1,7 +1,6 @@
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);
}
}
diff --git a/todolist/core/src/main/java/todolist/core/TodoSettingsListener.java b/todolist/core/src/main/java/todolist/core/TodoSettingsListener.java
index 3178b49..28698c4 100644
--- a/todolist/core/src/main/java/todolist/core/TodoSettingsListener.java
+++ b/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
diff --git a/todolist/core/src/main/java/todolist/json/internal/TodoSettingsDeserializer.java b/todolist/core/src/main/java/todolist/json/internal/TodoSettingsDeserializer.java
index d05316c..f6aee4d 100644
--- a/todolist/core/src/main/java/todolist/json/internal/TodoSettingsDeserializer.java
+++ b/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
}
diff --git a/todolist/core/src/main/java/todolist/json/internal/TodoSettingsSerializer.java b/todolist/core/src/main/java/todolist/json/internal/TodoSettingsSerializer.java
index b79dfeb..31bf770 100644
--- a/todolist/core/src/main/java/todolist/json/internal/TodoSettingsSerializer.java
+++ b/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();
}
}
diff --git a/todolist/core/src/test/java/todolist/core/TodoModelTest.java b/todolist/core/src/test/java/todolist/core/TodoModelTest.java
index 1bf7c69..e3fc9ac 100644
--- a/todolist/core/src/test/java/todolist/core/TodoModelTest.java
+++ b/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()]));
}
diff --git a/todolist/core/src/test/java/todolist/core/TodoSettingsTest.java b/todolist/core/src/test/java/todolist/core/TodoSettingsTest.java
index 0924cae..37822c0 100644
--- a/todolist/core/src/test/java/todolist/core/TodoSettingsTest.java
+++ b/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);
}
}
diff --git a/todolist/core/src/test/java/todolist/json/TodoModuleTest.java b/todolist/core/src/test/java/todolist/json/TodoModuleTest.java
index 2d67b2e..ab70ce6 100644
--- a/todolist/core/src/test/java/todolist/json/TodoModuleTest.java
+++ b/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());
}
diff --git a/todolist/fxui/src/main/java/todolist/ui/TodoItemListCellDragHandler.java b/todolist/fxui/src/main/java/todolist/ui/TodoItemListCellDragHandler.java
index 1db6937..5f6a425 100644
--- a/todolist/fxui/src/main/java/todolist/ui/TodoItemListCellDragHandler.java
+++ b/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;
diff --git a/todolist/fxui/src/main/java/todolist/ui/TodoModelController.java b/todolist/fxui/src/main/java/todolist/ui/TodoModelController.java
index 980f893..87b5658 100644
--- a/todolist/fxui/src/main/java/todolist/ui/TodoModelController.java
+++ b/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));
}
diff --git a/todolist/fxui/src/main/java/todolist/ui/TodoSettingsController.java b/todolist/fxui/src/main/java/todolist/ui/TodoSettingsController.java
index 6a2cf1a..71452ee 100644
--- a/todolist/fxui/src/main/java/todolist/ui/TodoSettingsController.java
+++ b/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
diff --git a/todolist/fxui/src/main/java/todolist/ui/util/SceneTarget.java b/todolist/fxui/src/main/java/todolist/ui/util/SceneTarget.java
index 15087ae..2388ae0 100644
--- a/todolist/fxui/src/main/java/todolist/ui/util/SceneTarget.java
+++ b/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;
diff --git a/todolist/rest/src/main/java/todolist/restapi/TodoSettingsResource.java b/todolist/rest/src/main/java/todolist/restapi/TodoSettingsResource.java
index 19e785b..b8b02a9 100644
--- a/todolist/rest/src/main/java/todolist/restapi/TodoSettingsResource.java
+++ b/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)
--
GitLab