Reduce complexity of private method FinishedJobs::findJobsToRemove

Also make it return an Iterable instead of null to avoid null checks
eclipse-platform · May 10, 2024 · afe3422 · afe3422
1 parent 29831f8
commit afe3422
Showing 1 changed file with 36 additions and 35 deletions.
diff --git a/...es/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/progress/FinishedJobs.java b/...es/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/progress/FinishedJobs.java
@@ -14,6 +14,7 @@
 package org.eclipse.ui.internal.progress;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.LinkedHashSet;
@@ -178,11 +179,8 @@ private void removeDuplicates(GroupInfo info) {
 
 	private void removeDuplicates(JobTreeElement info) {
 		synchronized (keptjobinfos) {
-			JobTreeElement[] toBeRemoved = findJobsToRemove(info);
-			if (toBeRemoved != null) {
-				for (JobTreeElement element : toBeRemoved) {
-					remove(element);
-				}
+			for (JobTreeElement element : findJobsToRemove(info)) {
+				remove(element);
 			}
 		}
 	}
@@ -231,53 +229,56 @@ static void disposeAction(JobTreeElement jte) {
 		}
 	}
 
-	private JobTreeElement[] findJobsToRemove(JobTreeElement info) {
-
-		if (info.isJobInfo()) {
-			Job myJob = ((JobInfo) info).getJob();
-
-			if (myJob != null) {
-
-				Object prop = myJob.getProperty(ProgressManagerUtil.KEEPONE_PROPERTY);
-				if (prop instanceof Boolean && ((Boolean) prop).booleanValue()) {
-					ArrayList<JobTreeElement> found = null;
-					JobTreeElement[] all = getKeptElements();
-					for (JobTreeElement jobTreeElement : all) {
-						if (jobTreeElement != info && jobTreeElement.isJobInfo()) {
-							Job job = ((JobInfo) jobTreeElement).getJob();
-							if (job != null && job != myJob && job.belongsTo(myJob)) {
-								if (found == null) {
-									found = new ArrayList<>();
-								}
-								found.add(jobTreeElement);
-							}
-						}
-					}
-					if (found != null) {
-						return found.toArray(new JobTreeElement[found.size()]);
-					}
+	private Iterable<JobTreeElement> findJobsToRemove(JobTreeElement info) {
+
+		if (!info.isJobInfo()) {
+			return Collections.emptyList();
+		}
+
+		Job myJob = ((JobInfo) info).getJob();
+
+		if (myJob == null) {
+			return Collections.emptyList();
+		}
+
+		if (!(myJob.getProperty(ProgressManagerUtil.KEEPONE_PROPERTY) instanceof Boolean keepOne) || !keepOne) {
+			return Collections.emptyList();
+		}
+
+		ArrayList<JobTreeElement> found = null;
+		JobTreeElement[] all = getKeptElements();
+		for (JobTreeElement jobTreeElement : all) {
+			if (jobTreeElement == info || !jobTreeElement.isJobInfo()) {
+				continue;
+			}
+			Job job = ((JobInfo) jobTreeElement).getJob();
+			if (job != null && job != myJob && job.belongsTo(myJob)) {
+				if (found == null) {
+					found = new ArrayList<>();
 				}
+				found.add(jobTreeElement);
 			}
 		}
-		return null;
+		if (found != null) {
+			return found;
+		}
+
+		return Collections.emptyList();
 	}
 
 	private void checkTasks(JobInfo info) {
 		if (keep(info)) {
 			Optional<TaskInfo> optional = info.getTaskInfo();
 			if (optional.isPresent()) {
 				TaskInfo tinfo = optional.get();
-				JobTreeElement[] toBeRemoved = null;
 				boolean fire = false;
 				JobInfo element = tinfo.getParent();
 				synchronized (keptjobinfos) {
 					if (element == info && !keptjobinfos.contains(tinfo)) {
-						toBeRemoved = findJobsToRemove(element);
+						Iterable<JobTreeElement> toBeRemoved = findJobsToRemove(element);
 						keptjobinfos.add(tinfo);
 						finishedTime.put(tinfo, Long.valueOf(System.currentTimeMillis()));
-					}
 
-					if (toBeRemoved != null) {
 						for (JobTreeElement jobTreeElement : toBeRemoved) {
 							remove(jobTreeElement);
 						}