From 5e16f7f37c984d7ee1d1f0484cf0a8154bbb849d Mon Sep 17 00:00:00 2001
From: Paul Buetow <paul@buetow.org>
Date: Fri, 20 Jun 2025 17:18:45 +0300
Subject: Improve code quality: Replace instanceof with polymorphism and
 extract constants
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Major improvements:
1. Replace instanceof checks with polymorphic methods in VSAbstractEvent hierarchy
   - Added isInternalEvent(), isMessageReceiveEvent(), etc. methods
   - Added getEventPriority() for clean event ordering
   - Added shouldIncreaseTimestamps() to control timestamp behavior
   - Refactored VSTask to use these polymorphic methods

2. Extract magic numbers and strings to constants
   - Created VSConstants class for centralized configuration values
   - Added event priority constants (PRIORITY_HIGHEST, PRIORITY_HIGH, etc.)
   - Extracted string constants like CLASS_PREFIX
   - Moved magic numbers to named constants (PERCENTAGE_RANGE, etc.)

3. Update tests to work with new polymorphic approach
   - Fixed mocking in VSTaskTest to return correct values
   - All 132 tests passing

These changes improve maintainability, reduce coupling, and make the codebase
more self-documenting. The polymorphic approach eliminates type checking and
makes it easier to add new event types.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
---
 src/main/java/core/VSInternalProcess.java |  2 +-
 src/main/java/core/VSTask.java            | 58 +++++--------------------------
 2 files changed, 10 insertions(+), 50 deletions(-)

(limited to 'src/main/java/core')

diff --git a/src/main/java/core/VSInternalProcess.java b/src/main/java/core/VSInternalProcess.java
index 81cc3fd..8116471 100644
--- a/src/main/java/core/VSInternalProcess.java
+++ b/src/main/java/core/VSInternalProcess.java
@@ -155,7 +155,7 @@ public class VSInternalProcess extends VSAbstractProcess {
      * @return A random percentage 0..100.
      */
     public synchronized int getRandomPercentage() {
-        return random.nextInt() % 101;
+        return random.nextInt() % constants.VSConstants.PERCENTAGE_RANGE;
     }
 
     /**
diff --git a/src/main/java/core/VSTask.java b/src/main/java/core/VSTask.java
index 54d7ff1..cc43f1c 100644
--- a/src/main/java/core/VSTask.java
+++ b/src/main/java/core/VSTask.java
@@ -6,11 +6,6 @@ import java.io.ObjectOutputStream;
 
 import events.VSAbstractEvent;
 import events.VSRegisteredEvents;
-import events.implementations.VSProcessCrashEvent;
-import events.implementations.VSProcessRecoverEvent;
-import events.internal.VSAbstractInternalEvent;
-import events.internal.VSMessageReceiveEvent;
-import events.internal.VSProtocolEvent;
 import exceptions.VSEventNotCopyableException;
 import prefs.VSPrefs;
 import protocols.VSAbstractProtocol;
@@ -165,7 +160,7 @@ public class VSTask implements Comparable<Object>, VSSerializable {
      * @return true, if the task is using an internal event
      */
     public boolean hasInternalEvent() {
-        return event instanceof VSAbstractInternalEvent;
+        return event.isInternalEvent();
     }
 
     /**
@@ -174,7 +169,7 @@ public class VSTask implements Comparable<Object>, VSSerializable {
      * @return true, if the task should not get serialized
      */
     public boolean hasNotSerializableEvent() {
-        return event instanceof VSNotSerializable;
+        return !event.isSerializable();
     }
 
     /**
@@ -183,7 +178,7 @@ public class VSTask implements Comparable<Object>, VSSerializable {
      * @return true, if it is a message receive event
      */
     public boolean hasMessageReceiveEvent() {
-        return event instanceof VSMessageReceiveEvent;
+        return event.isMessageReceiveEvent();
     }
 
     /**
@@ -192,7 +187,7 @@ public class VSTask implements Comparable<Object>, VSSerializable {
      * @return true, if it is a process recover event
      */
     public boolean hasProcessRecoverEvent() {
-        return event instanceof VSProcessRecoverEvent;
+        return event.isProcessRecoverEvent();
     }
 
     /**
@@ -268,8 +263,7 @@ public class VSTask implements Comparable<Object>, VSSerializable {
         if (event.getProcess() == null)
             event.init(process);
 
-        if (!(event instanceof VSMessageReceiveEvent)
-                && !(event instanceof VSAbstractProtocol))
+        if (event.shouldIncreaseTimestamps())
             process.increaseVectorAndLamportTimeIfAll();
 
         event.onStart();
@@ -370,44 +364,10 @@ public class VSTask implements Comparable<Object>, VSSerializable {
 
             VSAbstractEvent event2 = task.getEvent();
 
-            /* If it's a recovering, it should get handled very first */
-            boolean a = event instanceof VSProcessRecoverEvent;
-            boolean b = event2 instanceof VSProcessRecoverEvent;
-
-            if (a && b)
-                return 0;
-
-            if (a)
-                return -1;
-
-            if (b)
-                return 1;
-
-            /* If it's a crash, it should get handled second first */
-            a = event instanceof VSProcessCrashEvent;
-            b = event2 instanceof VSProcessCrashEvent;
-
-            if (a && b)
-                return 0;
-
-            if (a)
-                return -1;
-
-            if (b)
-                return 1;
-
-            /* If it's a VSProtocolEvent, it should get handled third  */
-            a = event instanceof VSProtocolEvent;
-            b = event2 instanceof VSProtocolEvent;
-
-            if (a && b)
-                return 0;
-
-            if (a)
-                return -1;
-
-            if (b)
-                return 1;
+            /* Use priority-based comparison for event ordering */
+            int priorityDiff = event.getEventPriority() - event2.getEventPriority();
+            if (priorityDiff != 0)
+                return priorityDiff;
 
             String shortname = event.getShortname();
             String shortname2 = event2.getShortname();
-- 
cgit v1.2.3