From e21fb72bd5dda558368ae63ee36c0b992d2412c7 Mon Sep 17 00:00:00 2001 From: Nadya Zabrodina Date: Wed, 14 Jun 2017 19:56:00 +0300 Subject: [PATCH] [ui]: add validation parameter to optimize addAll actions, cleanUp * it's better to add all actions simultaneously instead one by one to avoid array copy; * add an ability to avoid validation (do not check contains many times) --- .../actionSystem/DefaultActionGroup.java | 50 +++++++++++-------- 1 file changed, 30 insertions(+), 20 deletions(-) diff --git a/platform/platform-api/src/com/intellij/openapi/actionSystem/DefaultActionGroup.java b/platform/platform-api/src/com/intellij/openapi/actionSystem/DefaultActionGroup.java index 5b540f5a63f8..d6db42d23846 100644 --- a/platform/platform-api/src/com/intellij/openapi/actionSystem/DefaultActionGroup.java +++ b/platform/platform-api/src/com/intellij/openapi/actionSystem/DefaultActionGroup.java @@ -24,6 +24,7 @@ import org.jetbrains.annotations.Nullable; import java.util.Arrays; import java.util.Collection; +import java.util.HashSet; import java.util.List; /** @@ -45,6 +46,7 @@ import java.util.List; */ public class DefaultActionGroup extends ActionGroup { private static final Logger LOG = Logger.getInstance("#com.intellij.openapi.actionSystem.DefaultActionGroup"); + private static final String CANT_ADD_ITSELF = "Cannot add a group to itself"; /** * Contains instances of AnAction */ @@ -75,25 +77,33 @@ public class DefaultActionGroup extends ActionGroup { * @since 13.0 */ public DefaultActionGroup(@NotNull List actions) { - this(null, false); - addActions(actions); + this(null, actions); } - public DefaultActionGroup(@NotNull String name, @NotNull List actions) { + public DefaultActionGroup(@Nullable String name, @NotNull List actions) { + this(name, actions, true); + } + + public DefaultActionGroup(@Nullable String name, @NotNull List actions, boolean validate) { this(name, false); - addActions(actions); + addActions(actions, validate); } - private void addActions(@NotNull List actions) { - for (AnAction action : actions) { - add(action); - } - } - - public DefaultActionGroup(String shortName, boolean popup) { + public DefaultActionGroup(@Nullable String shortName, boolean popup) { super(shortName, popup); } + private void addActions(@NotNull List actions, boolean validate) { + if (validate) { + HashSet actionSet = new HashSet<>(); + for (AnAction action : actions) { + if (action == this) throw new IllegalArgumentException(CANT_ADD_ITSELF); + if (!(action instanceof Separator) && !actionSet.add(action)) throw new ActionDuplicationException(action); + } + } + mySortedChildren.addAll(actions); + } + /** * Adds the specified action to the tail. * @@ -142,18 +152,12 @@ public class DefaultActionGroup extends ActionGroup { } public final ActionInGroup addAction(@NotNull AnAction action, @NotNull Constraints constraint, @NotNull ActionManager actionManager) { - if (action == this) { - throw new IllegalArgumentException("Cannot add a group to itself"); - } + if (action == this) throw new IllegalArgumentException(CANT_ADD_ITSELF); // Check that action isn't already registered if (!(action instanceof Separator)) { - if (mySortedChildren.contains(action)) { - throw new IllegalArgumentException("cannot add an action twice: " + action); - } + if (mySortedChildren.contains(action)) throw new ActionDuplicationException(action); for (Pair pair : myPairs) { - if (action.equals(pair.first)) { - throw new IllegalArgumentException("cannot add an action twice: " + action); - } + if (action.equals(pair.first)) throw new ActionDuplicationException(action); } } @@ -409,4 +413,10 @@ public class DefaultActionGroup extends ActionGroup { public void addSeparator(@Nullable String separatorText) { add(new Separator(separatorText)); } + + private static class ActionDuplicationException extends IllegalArgumentException { + public ActionDuplicationException(@NotNull AnAction action) { + super("cannot add an action twice: " + action); + } + } }