From 54eb438e736a7112f657945a834ba399d5736878 Mon Sep 17 00:00:00 2001 From: Rothanak So Date: Thu, 27 Oct 2016 20:19:11 -0400 Subject: [PATCH 1/3] Fixed inconsistency in CommandGroup behavior when composing a subclassed instance, closes #81 --- .../strongback/command/CommandGroupTest.java | 47 ++++++++++++++++++- .../org/strongback/command/CommandGroup.java | 18 ++++--- .../src/org/strongback/command/Scheduler.java | 3 -- 3 files changed, 57 insertions(+), 11 deletions(-) diff --git a/strongback-tests/src/org/strongback/command/CommandGroupTest.java b/strongback-tests/src/org/strongback/command/CommandGroupTest.java index 0a0864e..95d10fb 100644 --- a/strongback-tests/src/org/strongback/command/CommandGroupTest.java +++ b/strongback-tests/src/org/strongback/command/CommandGroupTest.java @@ -332,6 +332,51 @@ public OtherDiagramFromBoard() { } } + @Test + public void shouldExecuteComposedNamedCommandGroups() { + scheduler.submit(new OuterCommandGroup()); + + assertThat(list).isEmpty(); + + // First step should come from the OuterCommandGroup + scheduler.execute(0); + assertThat(list).isEqualTo(listOf("C0 init", "C0 exe", "C0 fin" )); + list.clear(); + + // Second step should come from NestedCommandGroup + scheduler.execute(0); + assertThat(list).isEqualTo(listOf("C1 init", "C1 exe", "C1 fin" )); + list.clear(); + + // Third step should come from NestedCommandGroup + scheduler.execute(0); + assertThat(list).isEqualTo(listOf("C2 init", "C2 exe", "C2 fin" )); + list.clear(); + + // Fourth step should resume the OuterCommandGroup sequence + scheduler.execute(0); + assertThat(list).isEqualTo(listOf("C3 init", "C3 exe", "C3 fin" )); + list.clear(); + + // Fifth step is empty, nothing executed + scheduler.execute(0); + assertThat(list).isEmpty(); + } + + private final class OuterCommandGroup extends CommandGroup { + @SuppressWarnings("synthetic-access") + public OuterCommandGroup() { + sequentially(c[0], new InnerNamedCommandGroup(), c[3]); + } + + private final class InnerNamedCommandGroup extends CommandGroup { + @SuppressWarnings("synthetic-access") + public InnerNamedCommandGroup() { + sequentially(c[1], c[2]); + } + } + } + private static final class TestCommand extends Command { private static int commandID = 0; @@ -454,4 +499,4 @@ public String toString() { return "R" + number; } } -} \ No newline at end of file +} diff --git a/strongback/src/org/strongback/command/CommandGroup.java b/strongback/src/org/strongback/command/CommandGroup.java index 7ab3441..985d55e 100644 --- a/strongback/src/org/strongback/command/CommandGroup.java +++ b/strongback/src/org/strongback/command/CommandGroup.java @@ -131,7 +131,7 @@ static enum Type { SEQUENTIAL, PARRALLEL, FORK; } - private Command root; + private CommandGroup root; private final Command[] commands; private final Type type; @@ -151,15 +151,19 @@ private CommandGroup(Command[] commands, Type type) { } Type getType() { - return type; + if (root != null) { + return root.type; + } else { + return type; + } } Command[] getCommands() { - return commands; - } - - Command getRoot() { - return root; + if (root != null) { + return root.getCommands(); + } else { + return commands; + } } /** diff --git a/strongback/src/org/strongback/command/Scheduler.java b/strongback/src/org/strongback/command/Scheduler.java index bd695ea..97eb838 100644 --- a/strongback/src/org/strongback/command/Scheduler.java +++ b/strongback/src/org/strongback/command/Scheduler.java @@ -65,9 +65,6 @@ public void killAll() { */ public void submit(Command command) { if (command != null) { - if (command instanceof CommandGroup) { - command = ((CommandGroup) command).getRoot(); - } CommandRunner runner = buildRunner(command, null); commands.add(runner); } From 46dad6ed9ab9ce712adfa93c2ad3b27ae3664b46 Mon Sep 17 00:00:00 2001 From: Rothanak So Date: Thu, 27 Oct 2016 21:32:17 -0400 Subject: [PATCH 2/3] Simplified the if statements --- .../src/org/strongback/command/CommandGroupTest.java | 8 ++++---- .../src/org/strongback/command/CommandGroup.java | 12 ++---------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/strongback-tests/src/org/strongback/command/CommandGroupTest.java b/strongback-tests/src/org/strongback/command/CommandGroupTest.java index 95d10fb..fa43f30 100644 --- a/strongback-tests/src/org/strongback/command/CommandGroupTest.java +++ b/strongback-tests/src/org/strongback/command/CommandGroupTest.java @@ -16,16 +16,16 @@ package org.strongback.command; -import static org.fest.assertions.Assertions.assertThat; +import org.junit.Before; +import org.junit.Test; +import org.strongback.Logger; import java.util.Arrays; import java.util.LinkedList; import java.util.List; import java.util.Queue; -import org.junit.Before; -import org.junit.Test; -import org.strongback.Logger; +import static org.fest.assertions.Assertions.assertThat; public class CommandGroupTest { private Scheduler scheduler; diff --git a/strongback/src/org/strongback/command/CommandGroup.java b/strongback/src/org/strongback/command/CommandGroup.java index 985d55e..b4c7243 100644 --- a/strongback/src/org/strongback/command/CommandGroup.java +++ b/strongback/src/org/strongback/command/CommandGroup.java @@ -151,19 +151,11 @@ private CommandGroup(Command[] commands, Type type) { } Type getType() { - if (root != null) { - return root.type; - } else { - return type; - } + return root != null ? root.type : type; } Command[] getCommands() { - if (root != null) { - return root.getCommands(); - } else { - return commands; - } + return root != null ? root.getCommands() : commands; } /** From f93616e2e1a212d9e7e2cef8a5b096baa819ebc3 Mon Sep 17 00:00:00 2001 From: Rothanak So Date: Thu, 27 Oct 2016 21:43:33 -0400 Subject: [PATCH 3/3] Updated CONTRIBUTORS.txt --- CONTRIBUTORS.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index b104bc4..b473ee9 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -2,4 +2,5 @@ Braden Steffaniak Matthew Alonso Nathan Brown Randall Hauch -Zach Anderson \ No newline at end of file +Zach Anderson +Rothanak So \ No newline at end of file