Task cleanup

gselzer · gselzer · commit e488315805d5 · 2024-05-06T16:26:13.000-05:00
diff --git a/scijava-progress/src/main/java/org/scijava/progress/Progress.java b/scijava-progress/src/main/java/org/scijava/progress/Progress.java
@@ -184,7 +184,7 @@ public static void register(final Object progressible,
 		}
 		else {
 			// part of an existing execution hierarchy
-			t = parent.createSubtask(progressible, description);
+			t = new Task(progressible, parent, description);
 		}
 		deque.push(t);
 		// Ping Listeners about the registration of progressible
diff --git a/scijava-progress/src/main/java/org/scijava/progress/Task.java b/scijava-progress/src/main/java/org/scijava/progress/Task.java
@@ -57,10 +57,8 @@ public class Task {
 	/** Designates task completion */
 	private boolean completed = false;
 
-	private boolean hasElements = true;
-
-	/** Number of tasks utilized by the task */
-	private long tasks = 1;
+	/** Number of tasks (this task + any subtasks) contained within this task */
+	private long totalTasks = 1;
 
 	/** Progress in current stage */
 	private final AtomicLong current = new AtomicLong(0);
@@ -92,6 +90,9 @@ public Task(final Object progressible, //
 	{
 		this.progressible = progressible;
 		this.parent = parent;
+		if (this.parent != null) {
+			this.parent.subTasks.add(this);
+		}
 		this.description = description;
 	}
 
@@ -109,24 +110,12 @@ public void complete() {
 			}
 			throw new IllegalStateException(msg);
 		}
-		this.completed = true;
-		if (hasElements) {
+		else {
+			// For tasks with undefined progress, we simply update current to be max.
 			this.current.set(this.max.get());
+			this.totalTasks = subTasks.size() + 1;
 		}
-	}
-
-	/**
-	 * Creates a subtask, recording it to keep track of progress.
-	 *
-	 * @return the subtask.
-	 */
-	public synchronized Task createSubtask( //
-		Object progressible, //
-		String description //
-	) {
-		final Task sub = new Task(progressible, this, description);
-		subTasks.add(sub);
-		return sub;
+		this.completed = true;
 	}
 
 	/**
@@ -157,25 +146,24 @@ public void defineTotal(final long elements) {
 	 *          used (as one subtask may run multiple times).
 	 */
 	public void defineTotal(final long elements, final long subTasks) {
-		if (tasksDefined) {
-			throw new IllegalStateException(
-				"Progress has already been defined for this task");
+		// Nonzero number of elements
+		if (elements > 0L) {
+			this.totalTasks = subTasks + 1;
+			this.max.set(elements);
 		}
-		// Total tasks = all subtasks, plus one iff there are also progress elements
-		// in this stage
-		this.tasks = subTasks + (elements == 0 ? 0 : 1);
-
-		if (elements == 0) {
-			this.hasElements = false;
-			if (this.tasks == 0) {
-				this.tasks = 1;
-				this.current.set(1);
-				this.max.set(1);
-				return;
-			}
+		// Zero elements, nonzero subtasks
+		else if (subTasks > 0L) {
+			this.totalTasks = subTasks;
+			this.max.set(1);
 		}
+		// Zero elements & zero subtasks
+		// NB one common example of this situation occurs when you
+		// want to programmatically set the elements to the size of a list,
+		// but then you get passed an empty list. In this case, there is one task,
+		// which is already complete!.
 		else {
-			this.max.set(elements);
+			this.totalTasks = 1;
+			this.current.set(1);
 		}
 		this.tasksDefined = true;
 	}
@@ -202,9 +190,9 @@ public boolean isComplete() {
 	public double progress() {
 		double totalCompletion = current.doubleValue() / max.doubleValue();
 		for (Task t : subTasks) {
-			totalCompletion += t.isComplete() ? 1.0 : t.progress();
+			totalCompletion += t.progress();
 		}
-		return totalCompletion / tasks;
+		return totalCompletion / totalTasks;
 	}
 
 	/**

Original file line number	Diff line number	Diff line change
`@@ -184,7 +184,7 @@ public static void register(final Object progressible,`
`184`	`184`	`}`
`185`	`185`	`else {`
`186`	`186`	`// part of an existing execution hierarchy`
`187`		`- t = parent.createSubtask(progressible, description);`
	`187`	`+ t = new Task(progressible, parent, description);`
`188`	`188`	`}`
`189`	`189`	`deque.push(t);`
`190`	`190`	`// Ping Listeners about the registration of progressible`