Replace Java 1.4 era 'for' loops by torusrxxx · Pull Request #1100 · hyphanet/fred

torusrxxx · 2025-08-18T14:46:14Z

This is a replacement for a previous pull request. Review suggestions have been addressed.
However, the code style and white-spaces review is so boring. Why didn't we have an automated tool that fixes it once and for all?

Use Enhanced 'for' loops

# Conflicts: # src/freenet/clients/http/WelcomeToadlet.java # src/freenet/config/StringArrOption.java # src/freenet/support/PrioritizedSerialExecutor.java

Bombe · 2025-08-22T05:38:25Z

However, the code style and white-spaces review is so boring. Why didn't we have an automated tool that fixes it once and for all?

Because we have 50+ open pull requests, and every single one of them would be fucked if we did that.

Bombe

Some of the variable names you introduced could be improved, too, e.g. the s in String s : methodNamesArray could incredibly well be named methodName to make it more clear what the variable does actually contain.

Bombe · 2025-08-22T08:28:52Z

src/freenet/clients/http/bookmark/BookmarkManager.java

+						if (logMINOR) Logger.minor(this, "Updating bookmark for " + furi + " to edition " + edition);
 						matched = true;
-						BookmarkItem item = items.get(i);
+						BookmarkItem item = bookmarkItem;


No, please don’t do this, use bookmarkItem instead of item in the line below. 🙂

Bombe · 2025-08-22T08:31:14Z

src/freenet/clients/http/ConnectionsToadlet.java

 				Integer prev = results.get(result);
-				if(prev == null) prev = 0;
-				results.put(result, prev+1);
+				if (prev == null) prev = Integer.valueOf(0);


No manual boxing, please.

Bombe · 2025-08-22T08:47:02Z

src/freenet/clients/http/StatisticsToadlet.java

-			if (a[i] > max) max = a[i];
-		}
+		int max;
+		max = Integer.max(Arrays.stream(a).reduce(Integer::max).orElse(1), 1);


This method does not need to be more than a single line. The declaration does not need to be separate, and the variable can be inlined directly.

Bombe · 2025-08-22T08:49:50Z

src/freenet/clients/http/StatisticsToadlet.java

@@ -1530,26 +1528,26 @@ private void addPeerCircle (HTMLNode circleTable, PeerNodeStatus[] peerNodeStatu
 		double peerLocation;
 		int histogramIndex;
 		int peerCount = peerNodeStatuses.length;


peerCount is now unused and can be removed.

Bombe · 2025-08-22T08:50:59Z

src/freenet/clients/http/StatisticsToadlet.java

-		for (int peerIndex = 0; peerIndex < peerCount; peerIndex++) {
-			peerNodeStatus = peerNodeStatuses[peerIndex];
+		for (PeerNodeStatus nodeStatus : peerNodeStatuses) {
+			peerNodeStatus = nodeStatus;


Don’t introduce these kind of alias variables, please. Either use the name from the old loop in the for statement, or change all the variables in the loop to what you used in the new for statement.

Bombe · 2025-08-22T09:20:05Z

test/freenet/support/LRUQueueTest.java

 		Object[] sampleObjects = createSampleObjects(size);
-		for (int i=0;i<sampleObjects.length;i++)
-			methodLRUQueue.push(sampleObjects[i]);
+		for (Object sampleObject : sampleObjects) methodLRUQueue.push(sampleObject);


Multiple lines and braces, please. For the remainder of this file, too.

Bombe · 2025-08-22T09:20:14Z

test/freenet/support/LRUQueueTest.java

 	private boolean isPresent(Object[] anArray, Object aElementToSearch) {
-		for(int i=0; i<anArray.length; i++)
-			if (anArray[i].equals(aElementToSearch))
+		for (Object o : anArray)


Braces, please.

Bombe · 2025-08-22T09:23:28Z

test/freenet/support/SimpleFieldSetTest.java

-			methodSFS.putSingle(aPairsArray[i][0], aPairsArray[i][1]);
-		for (int i = 0; i < aPairsArray.length; i++)		//getting values
-			retValue &= methodSFS.get(aPairsArray[i][0]).equals(aPairsArray[i][1]);
+		for (String[] value : aPairsArray) methodSFS.putSingle(value[0], value[1]);


Huh, this pull request started quite well in the “multiple lines and braces” area, but it seems to degrade more and more? I will not comment this on every of the remaining files where this occurs but will mention it here once for the remainder of this pull request: when you change code that either lumps up multiple statements in one line, or that uses multiple lines with identation, but without braces, change that code to be multiple lines (one line per statement) and to have braces around each code block.

Bombe · 2025-08-22T09:43:54Z

test/freenet/support/TimeUtilTest.java

-	}
-
-	@Test
-	public void testToMillis_fractionalMillis() {


And where did this test go?

Automatic merging seems to mess up with this one and the next one.

Bombe · 2025-08-22T09:46:04Z

test/freenet/support/TimeUtilTest.java

+		try {
+			TimeUtil.toMillis("15250284452w3q7h12m55.807s");
+		} catch (NumberFormatException e) {
+				assertNotNull(e);


Uhm, no. a) This doesn’t even perform the check correctly, i.e. if toMillis does not throw, this test is still green. b) assertThrows is the perfect way to verify that a method throws an exception. c) The exception captured in a catch is always non-null, that assertNotNull is useless.

This was probably written before assertThrows is supported. Many test cases in this test suite have been updated to use assertThrows.

…yle for tests

torusrxxx · 2025-08-30T14:19:00Z

However, the code style and white-spaces review is so boring. Why didn't we have an automated tool that fixes it once and for all?

Because we have 50+ open pull requests, and every single one of them would be fucked if we did that.

30+ open pull requests are older than 2 years, and every single one of them already needs complete rework to get merged.

Address code review feedback from PR hyphanet#1100: - Replace single-letter variables with descriptive names: - s -> metaString, headerValue, nodeReference, octet, nodeName - r -> requestStatus - f -> file - a/b -> nodeA/nodeB - Replace generic 'item' with specific 'bookmarkItem' Based on PR hyphanet#1100 by Juiceman and torusrxxx 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> Co-Authored-By: Juiceman <juiceman@hyphanet.org> Co-Authored-By: torusrxxx <torusrxxx@users.noreply.github.com>

Juiceman and others added 7 commits March 17, 2024 21:25

Replace Java 1.4 era 'for' loops

b44597f

Use Enhanced 'for' loops

Merge branch 'next' into for-loops

d05005c

Replace Java 1.4 era 'for' loops

6c07706

Use Enhanced 'for' loops

Merge remote-tracking branch 'Juiceman-origin/for-loops' into for-loops

ee34184

# Conflicts: # src/freenet/clients/http/WelcomeToadlet.java # src/freenet/config/StringArrOption.java # src/freenet/support/PrioritizedSerialExecutor.java

Merge branch 'next' into for-loops

c1ca629

Replace some for loops with stream

ec623ce

Add braces to more statements

349e1f4

Bombe requested changes Aug 22, 2025

View reviewed changes

torusrxxx added 2 commits August 22, 2025 20:15

Merge branch 'next' into for-loops

32d4564

Add braces to single line conditional statements and optimize code st…

ea6a03f

…yle for tests

torusrxxx force-pushed the for-loops branch from cacd6e5 to ea6a03f Compare August 22, 2025 15:40

Bombe mentioned this pull request Aug 30, 2025

Replace Java 1.4 era 'for' loops #955

Open

Conversation

torusrxxx commented Aug 18, 2025

Uh oh!

Bombe commented Aug 22, 2025

Uh oh!

Bombe left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

torusrxxx commented Aug 30, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants