Migrate to Lambda `forEach` from Traditional `for` Loop

[Pankraz76]

✨ Migrate to Lambda forEach from Traditional for Loop

Migrating from traditional for loops to lambda-based forEach improves code readability and aligns with modern Java functional programming practices.

✅ In this case, we reduced 75% overhead, going from 4 LOC to just 1 LOC — a clear win in simplicity and maintainability.

---

🚀 Benefits of this change:

• ✅ Cleaner, more concise code (often reducing to a single line)
• ☕ Better use of the Java Streams API
• 🔗 Improved cohesion and reduced coupling
• 🧼 Follows clean code principles and promotes functional design

---

🔗 Pull Request: #16874
📸

---

🔧 Refactor Origin

This change is a combination of two improvements:

• S1488: Local variables should not be declared and then immediately returned or thrown
  📌 Related Issue: #16488

• Replacing old-school for loops with a modern .forEach lambda pattern

---

🧠 Code Example

// ❌ Before: Traditional for-loop
for (Item item : items) {
    process(item);
}

// ✅ After: Lambda forEach
items.forEach(item -> process(item));

📉 This change significantly reduces boilerplate while preserving the original behavior.

---

🔗 References

• PR: https://github.com/checkstyle/checkstyle/pull/16874
• Related Rule: https://github.com/checkstyle/checkstyle/pull/16488

[ViliqnVachev]

Is this applicable to the entire project?

[mkarg]

Always inlining the event creation could lead to performance drawbacks when there are many listeners, and hence the variable be an intention of the original author. OpenRewrite should be more careful due to that and not remove the variable.

[Pankraz76]

Thanks for the valid and significant note. This is indeed something to consider. Also the example and the actual code diff. are two different kind, only one having this potential drawback.

So when doing something with new operator or calling some method(), this could lead to some performance differences.

Still I would like to see the bare numbers in order of RulesOfOptimization, not so solve some tiny edge case in sake of large scale.

[mkarg]

As performance of new is tightly bound to the constructor's implementation (and all other initializers), it is impossible to give a general number. Construction might range from zero to multiple seconds in the worst case. Hence, inlining should nevern happen automatically, but only when the operator explicitly asks for it in a specific case.

[Pankraz76]

but only when the operator explicitly asks for it in a specific case.

Yes, I would generally agree with inverting that approach and making configuration the convention—leveraging the power of convention principle.

That said, in my opinion this should be the exception rather than the rule, unlike how it’s currently handled in many codebases.

If there is a high payload or a similar constraint, it should be explicitly marked as dedicated—much like the occasionally used (technically unused but intentional) fields that establish a clear SSOT. This makes it easy to distinguish what is truly mandatory from what is merely unnecessary coupling. Most parts should be inlined by default and not treated as critical performance code, especially in the context we’re discussing. This isn’t the 80s anymore, when performance was measured in kilobytes.

Code conventions and simplicity should be the primary drivers. My proposal should satisfy both of us: inline the routine or boilerplate parts by default, and extract only the few important fields that genuinely need special handling—clearly marked with something like a // noinspection or similar annotation. There is almost always a way to establish a clear convention instead of perpetuating conventions born from confusion.

So, I would prefer inlining everything by default, supported by static analysis, and explicitly marking the small number of truly important exceptions. Those exceptions should clearly document why they matter, which is essentially the same point you’re making. That’s why it feels like we’re largely aligned, even if we’re approaching the problem from slightly different angles.

[mkarg]

I see your point, but as this discussion proofs, there are good arguments for both outcomes: Inlining it vs not inlining it. Even the author's personal choice is one of these arguments: Do we want to force them to explicitly safeguard themselves from getting forcecully pushed into your personal choice (which feels like an unwanted and non-beneficial premature optimization to them)? IMHO OpenRewrite is just a tool, so it should be the control of the operator what rules it applies and which configuration choices it defaults. Reading your message I somewhat have the impression that you want the world to accept your personal preferences. I do not think that this a message we should send. Inlining things which were explicitly not inlined previously definitively is breaking the original authors personal choice and style (and even could be backwards incompatible: in case the event constructor adds the current timestamp to the message, all events have divergent hashes then), so we should be very careful, as the actual benefit you gain is in fact doubtful. Having said that, I would prefer making the inlining a separate rule, so the operator can adopt it explicitly, without being forced to explcititly preventing it-

[Pankraz76]

Yes, sorry—I’m only realizing this now. I assumed it was some kind of new recipe idea from my past. My apologies.

Of course, any kind of migration is nearly impossible to carry out; that’s the lesson I’ve learned.

personal preferences

The whole point of this project is the principle of convention—specifically, avoiding anything personal (=config) upfront.

That’s my anti-goal: the world simply doesn’t like to admit conventions, because doing so would reduce chaos—which is seen as impossible and, in a way, unnatural.

Still, thank you for the strong arguments you made.