From: "Enrico Weigelt, metux IT consult" <enrico.weig...@gr13.net>

Retrieval of currently movable units semantically fits better into
the Player object instead of outer business logic.

Also simplifying the code flow by using a plain loop instead of
lamdas and Stream magic - the benefit of potential parallelization
is pretty hypothetical here, as the tests are pretty trivial,
compared to the overhead of Streams w/ lamdas (IOW: callbacks).

OTOH, we could optimize further by not using getters internally
within Unit or at least make them final, so the compiler can
inline them.
---
 src/net/sf/freecol/client/control/InGameController.java |  8 +++++++-
 src/net/sf/freecol/common/model/Player.java             | 11 +++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/src/net/sf/freecol/client/control/InGameController.java 
b/src/net/sf/freecol/client/control/InGameController.java
index 9d2f6239f91..5380aec31f4 100644
--- a/src/net/sf/freecol/client/control/InGameController.java
+++ b/src/net/sf/freecol/client/control/InGameController.java
@@ -778,7 +778,13 @@ public final class InGameController extends 
FreeColClientHolder {
     private boolean doEndTurn(boolean showDialog) {
         final Player player = getMyPlayer();
         if (showDialog) {
-            List<Unit> units = transform(player.getUnits(), Unit::couldMove);
+            /** we could check for movable units first, before retrieving
+                a list of them - that would save unnecessary list allocation
+                in case there none left, but at the cost of having to loop
+                through the list twice, if there are some. the worst case
+                would be just the last unit in the list is movable - then
+                we'd end up w/ two full list scans instead of just one. **/
+            List<Unit> units = player.getMovableUnits();
             if (!units.isEmpty()) {
                 // Modal dialog takes over
                 getGUI().showEndTurnDialog(units,
diff --git a/src/net/sf/freecol/common/model/Player.java 
b/src/net/sf/freecol/common/model/Player.java
index 5573d56cc4b..0da6e80228f 100644
--- a/src/net/sf/freecol/common/model/Player.java
+++ b/src/net/sf/freecol/common/model/Player.java
@@ -1923,6 +1923,17 @@ public class Player extends FreeColGameObject implements 
Nameable {
     }
 
     /**
+     * Retrieve a list of still movable units
+     */
+    public final List<Unit> getMovableUnits() {
+        List<Unit> result = new ArrayList<>();
+        for (Unit u : this.units)
+            if (u.couldMove())
+                result.add(u);
+        return result;
+    }
+
+    /**
      * Get the number of units a player has.
      *
      * @return The number of units.
-- 
2.11.0.rc0.7.gbe5a750


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Freecol-developers mailing list
Freecol-developers@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/freecol-developers

Reply via email to