From 121d4cd44adacefa4fc5b3173146652589d38dd0 Mon Sep 17 00:00:00 2001
From: Berack96 <giacomobertolazzi7@gmail.com>
Date: Wed, 19 Feb 2025 11:06:07 +0100
Subject: [PATCH] Refactor Event comparison logic for clarity and add
 getRandomChild method to ServerNodeState for improved child selection; update
 simulation tests for null checks and event processing

---
 .../java/net/berack/upo/valpre/sim/Event.java |  6 +----
 .../upo/valpre/sim/ServerNodeState.java       | 26 ++++++++++++++-----
 .../net/berack/upo/valpre/sim/Simulation.java |  5 ++++
 .../berack/upo/valpre/sim/TestSimulation.java | 17 ++++++++++--
 4 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/src/main/java/net/berack/upo/valpre/sim/Event.java b/src/main/java/net/berack/upo/valpre/sim/Event.java
index 91d6fb2..cd8c9f7 100644
--- a/src/main/java/net/berack/upo/valpre/sim/Event.java
+++ b/src/main/java/net/berack/upo/valpre/sim/Event.java
@@ -23,11 +23,7 @@ public class Event implements Comparable<Event> {
 
     @Override
     public int compareTo(Event other) {
-        if (this.time < other.time)
-            return -1;
-        if (this.time == other.time)
-            return 0;
-        return 1;
+        return Double.compare(this.time, other.time);
     }
 
     /**
diff --git a/src/main/java/net/berack/upo/valpre/sim/ServerNodeState.java b/src/main/java/net/berack/upo/valpre/sim/ServerNodeState.java
index 2ada237..6734e10 100644
--- a/src/main/java/net/berack/upo/valpre/sim/ServerNodeState.java
+++ b/src/main/java/net/berack/upo/valpre/sim/ServerNodeState.java
@@ -158,6 +158,23 @@ public class ServerNodeState {
         return null;
     }
 
+    /**
+     * Get a random child based on the weights of the children and the random number
+     * generator passed as input
+     * 
+     * @param rng the random number generator
+     * @return the index of the child or -1 if no child is selected
+     */
+    public int getRandomChild(Rng rng) {
+        var random = rng.random();
+        for (var child : this.children) {
+            random -= child.weight;
+            if (random <= 0)
+                return child.index;
+        }
+        return -1;
+    }
+
     /**
      * Create an arrival event to a child node based on the node and the time passed
      * as input
@@ -168,12 +185,7 @@ public class ServerNodeState {
      *         otherwise
      */
     public Event spawnArrivalToChild(double time, Rng rng) {
-        var random = rng.random();
-        for (var child : this.children) {
-            random -= child.weight;
-            if (random <= 0)
-                return Event.newArrival(child.index, time);
-        }
-        return null;
+        var child = this.getRandomChild(rng);
+        return child > -1 ? Event.newArrival(child, time) : null;
     }
 }
\ No newline at end of file
diff --git a/src/main/java/net/berack/upo/valpre/sim/Simulation.java b/src/main/java/net/berack/upo/valpre/sim/Simulation.java
index 0941412..861bc4e 100644
--- a/src/main/java/net/berack/upo/valpre/sim/Simulation.java
+++ b/src/main/java/net/berack/upo/valpre/sim/Simulation.java
@@ -97,8 +97,13 @@ public final class Simulation {
             case DEPARTURE -> {
                 state.updateDeparture(time);
 
+                // Spawn unavailability if has unavailable time
                 this.addToFel(state.spawnUnavailableIfPossible(time, this.rng));
+
+                // Spawn departure if has requests and server is available
                 this.addToFel(state.spawnDepartureIfPossible(time, this.rng));
+
+                // Spawn arrival to self if is source node
                 this.addToFel(state.spawnArrivalIfPossilbe(time));
 
                 // Spawn arrival to child node if queue is not full otherwise drop
diff --git a/src/test/java/net/berack/upo/valpre/sim/TestSimulation.java b/src/test/java/net/berack/upo/valpre/sim/TestSimulation.java
index 82fb462..5f2c317 100644
--- a/src/test/java/net/berack/upo/valpre/sim/TestSimulation.java
+++ b/src/test/java/net/berack/upo/valpre/sim/TestSimulation.java
@@ -5,6 +5,7 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import java.util.HashSet;
 
@@ -393,8 +394,18 @@ public class TestSimulation {
 
     @Test
     public void simulation() {
-        var start = System.nanoTime();
+        assertThrows(NullPointerException.class, () -> new Simulation(null, rigged));
+        assertThrows(NullPointerException.class, () -> new Simulation(simpleNet, null));
+
         var sim = new Simulation(simpleNet, rigged);
+        assertTrue(sim.hasEnded());
+        assertEquals(0, sim.getEventsProcessed());
+        assertEquals(0.0, sim.getTime(), DELTA);
+        var fel = sim.getFutureEventList();
+        assertEquals(0, fel.size());
+
+        var start = System.nanoTime();
+        sim = new Simulation(simpleNet, rigged);
         // knowing that it takes time to allocate the object
         // we can use the average time
         var endAllocation = System.nanoTime();
@@ -410,7 +421,7 @@ public class TestSimulation {
         assertEquals(0, sim.getNodeState(node0.name).numServerUnavailable);
         assertEquals(0, sim.getNodeState(node1.name).numServerBusy);
         assertEquals(0, sim.getNodeState(node1.name).numServerUnavailable);
-        var fel = sim.getFutureEventList();
+        fel = sim.getFutureEventList();
         assertEquals(0, fel.size());
 
         sim.addToFel(Event.newArrival(0, sim.getTime()));
@@ -489,6 +500,8 @@ public class TestSimulation {
         assertEquals(0, sim.getNodeState(node1.name).numServerUnavailable);
         fel = sim.getFutureEventList();
         assertEquals(0, fel.size());
+        final var s = sim;
+        assertThrows(NullPointerException.class, () -> s.processNextEvent());
 
         var elapsed = (double) (System.nanoTime() - time);
         var result = sim.endSimulation();