Remove Logger.assertTrue.

Use Guava's Preconditions instead, which throw appropriate exceptions
rather than AssertionErrors (whose idea was that?!)

Also fix weirdness in CommandArguments where start was allowed to be
1 greater than end when getting words.

Change-Id: Icce38e36da6507f645fb8b3b872dfe2c60c43ea7
Reviewed-on: http://gerrit.dmdirc.com/2961
Reviewed-by: Greg Holmes <greg@dmdirc.com>
Automatic-Compile: DMDirc Build Manager

src/com/dmdirc/FrameContainerComparator.java

@@ -22,16 +22,15 @@
 
 package com.dmdirc;
 
-import com.dmdirc.logger.Logger;
-
 import java.io.Serializable;
 import java.util.Comparator;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 /**
  * Compares FrameContainers by name.
  */
-public final class FrameContainerComparator implements Comparator<FrameContainer>,
-        Serializable {
+public class FrameContainerComparator implements Comparator<FrameContainer>, Serializable {
 
     /**
      * A version number for this class. It should be changed whenever the class
@@ -56,10 +55,10 @@ public final class FrameContainerComparator implements Comparator<FrameContainer
         "item2.toString() returns a non-null value"
     })
     public int compare(final FrameContainer item1, final FrameContainer item2) {
-        Logger.assertTrue(item1 != null);
-        Logger.assertTrue(item2 != null);
-        Logger.assertTrue(item1.getName() != null);
-        Logger.assertTrue(item2.getName() != null);
+        checkNotNull(item1);
+        checkNotNull(item2);
+        checkNotNull(item1.getName());
+        checkNotNull(item2.getName());
 
         if (sortBefore(item1, item2)) {
             return -1;
@@ -84,9 +83,9 @@ public final class FrameContainerComparator implements Comparator<FrameContainer
      * @param item2 The existing container to test against
      * @return True iff the new container should be before the old container
      */
-    private static boolean sortBefore(final FrameContainer item1,
+    private static boolean sortBefore(
+            final FrameContainer item1,
             final FrameContainer item2) {
-
         return getPosition(item1) < getPosition(item2);
     }
 

src/com/dmdirc/Server.java

@@ -87,6 +87,9 @@ import javax.net.ssl.TrustManager;
 
 import lombok.extern.slf4j.Slf4j;
 
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+
 /**
  * The Server class represents the client's view of a server. It maintains
  * a list of all channels, queries, etc, and handles parser callbacks pertaining
@@ -830,8 +833,8 @@ public class Server extends WritableFrameContainer implements ConfigChangeListen
         "The current profile specifies at least one nickname"
     })
     private MyInfo buildMyInfo() {
-        Logger.assertTrue(profile != null);
-        Logger.assertTrue(!profile.getOptionList(DOMAIN_PROFILE, "nicknames").isEmpty());
+        checkNotNull(profile);
+        checkArgument(!profile.getOptionList(DOMAIN_PROFILE, "nicknames").isEmpty());
 
         final MyInfo myInfo = new MyInfo();
         myInfo.setNickname(profile.getOptionList(DOMAIN_PROFILE, "nicknames").get(0));

src/com/dmdirc/ServerManager.java

@@ -45,6 +45,8 @@ import javax.inject.Inject;
 import javax.inject.Provider;
 import javax.inject.Singleton;
 
+import static com.google.common.base.Preconditions.checkArgument;
+
 /**
  * The ServerManager maintains a list of all servers, and provides methods to
  * search or iterate over them.
@@ -210,7 +212,7 @@ public class ServerManager implements ServerFactory {
      * @since 0.6.3
      */
     public Server connectToAddress(final URI uri, final ConfigProvider profile) {
-        Logger.assertTrue(profile.isProfile());
+        checkArgument(profile.isProfile());
         Server server = null;
 
         for (Server loopServer : servers) {

src/com/dmdirc/actions/ActionComponentChain.java

@@ -25,11 +25,12 @@ package com.dmdirc.actions;
 import com.dmdirc.Precondition;
 import com.dmdirc.interfaces.ActionController;
 import com.dmdirc.interfaces.actions.ActionComponent;
-import com.dmdirc.logger.Logger;
 
 import java.util.ArrayList;
 import java.util.List;
 
+import static com.google.common.base.Preconditions.checkArgument;
+
 /**
  * An action component chain supports chaining of multiple action components
  * together.
@@ -89,7 +90,7 @@ public class ActionComponentChain implements ActionComponent {
     @Precondition("This component chain has one or more components")
     @Override
     public Class<?> appliesTo() {
-        Logger.assertTrue(!components.isEmpty());
+        checkArgument(!components.isEmpty());
 
         return components.get(0).appliesTo();
     }
@@ -98,7 +99,7 @@ public class ActionComponentChain implements ActionComponent {
     @Precondition("This component chain has one or more components")
     @Override
     public Class<?> getType() {
-        Logger.assertTrue(!components.isEmpty());
+        checkArgument(!components.isEmpty());
 
         return components.get(components.size() - 1).getType();
     }
@@ -107,7 +108,7 @@ public class ActionComponentChain implements ActionComponent {
     @Precondition("This component chain has one or more components")
     @Override
     public String getName() {
-        Logger.assertTrue(!components.isEmpty());
+        checkArgument(!components.isEmpty());
 
         final StringBuilder name = new StringBuilder();
 
@@ -123,7 +124,7 @@ public class ActionComponentChain implements ActionComponent {
     @Override
     @Precondition("This component chain has one or more components")
     public String toString() {
-        Logger.assertTrue(!components.isEmpty());
+        checkArgument(!components.isEmpty());
 
         final StringBuilder name = new StringBuilder();
 

src/com/dmdirc/actions/ActionGroup.java

@@ -24,7 +24,6 @@ package com.dmdirc.actions;
 
 import com.dmdirc.Precondition;
 import com.dmdirc.config.prefs.PreferencesSetting;
-import com.dmdirc.logger.Logger;
 import com.dmdirc.updater.Version;
 
 import java.util.ArrayList;
@@ -33,6 +32,9 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+
 /**
  * Represents a group of actions, along with their meta-data.
  */
@@ -230,8 +232,8 @@ public class ActionGroup implements Iterable<Action> {
     })
     @SuppressWarnings("deprecation")
     public void deleteAction(final Action action) {
-        Logger.assertTrue(action != null);
-        Logger.assertTrue(actions.contains(action));
+        checkNotNull(action);
+        checkArgument(actions.contains(action));
 
         ActionManager.getActionManager().removeAction(action);
         action.delete();

src/com/dmdirc/actions/ActionManager.java

@@ -34,7 +34,6 @@ import com.dmdirc.interfaces.actions.ActionType;
 import com.dmdirc.interfaces.config.IdentityController;
 import com.dmdirc.logger.ErrorLevel;
 import com.dmdirc.logger.Logger;
-import com.dmdirc.updater.UpdateChecker;
 import com.dmdirc.updater.components.ActionGroupComponent;
 import com.dmdirc.updater.manager.UpdateManager;
 import com.dmdirc.util.collections.MapList;
@@ -53,6 +52,9 @@ import javax.inject.Provider;
 
 import lombok.extern.slf4j.Slf4j;
 
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+
 /**
  * Manages all actions for the client.
  */
@@ -215,7 +217,7 @@ public class ActionManager implements ActionController {
     @Override
     public void registerTypes(final ActionType[] newTypes) {
         for (ActionType type : newTypes) {
-            Logger.assertTrue(type != null);
+            checkNotNull(type);
 
             if (!types.contains(type)) {
                 log.debug("Registering action type: {}", type);
@@ -229,7 +231,7 @@ public class ActionManager implements ActionController {
     @Override
     public void registerComponents(final ActionComponent[] comps) {
         for (ActionComponent comp : comps) {
-            Logger.assertTrue(comp != null);
+            checkNotNull(comp);
 
             log.debug("Registering action component: {}", comp);
             components.add(comp);
@@ -240,7 +242,7 @@ public class ActionManager implements ActionController {
     @Override
     public void registerComparisons(final ActionComparison[] comps) {
         for (ActionComparison comp : comps) {
-            Logger.assertTrue(comp != null);
+            checkNotNull(comp);
 
             log.debug("Registering action comparison: {}", comp);
             comparisons.add(comp);
@@ -313,8 +315,8 @@ public class ActionManager implements ActionController {
      */
     @Precondition("The specified File is not null and represents a directory")
     private void loadActions(final File dir) {
-        Logger.assertTrue(dir != null);
-        Logger.assertTrue(dir.isDirectory());
+        checkNotNull(dir);
+        checkArgument(dir.isDirectory());
 
         log.debug("Loading actions from directory: {}", dir.getAbsolutePath());
 
@@ -330,7 +332,7 @@ public class ActionManager implements ActionController {
     /** {@inheritDoc} */
     @Override
     public void addAction(final Action action) {
-        Logger.assertTrue(action != null);
+        checkNotNull(action);
 
         log.debug("Registering action: {}/{} (status: {})",
                 new Object[] { action.getGroup(), action.getName(), action.getStatus() });
@@ -358,7 +360,7 @@ public class ActionManager implements ActionController {
     /** {@inheritDoc} */
     @Override
     public void removeAction(final Action action) {
-        Logger.assertTrue(action != null);
+        checkNotNull(action);
 
         actions.removeFromAll(action);
         getOrCreateGroup(action.getGroup()).remove(action);
@@ -375,9 +377,9 @@ public class ActionManager implements ActionController {
     @Override
     public boolean triggerEvent(final ActionType type,
             final StringBuffer format, final Object ... arguments) {
-        Logger.assertTrue(type != null);
-        Logger.assertTrue(type.getType() != null);
-        Logger.assertTrue(type.getType().getArity() == arguments.length);
+        checkNotNull(type);
+        checkNotNull(type.getType());
+        checkArgument(type.getType().getArity() == arguments.length);
 
         log.trace("Calling listeners for event of type {}", type);
 
@@ -413,7 +415,7 @@ public class ActionManager implements ActionController {
     @Precondition("The specified ActionType is not null")
     private boolean triggerActions(final ActionType type,
             final StringBuffer format, final Object ... arguments) {
-        Logger.assertTrue(type != null);
+        checkNotNull(type);
 
         boolean res = false;
 
@@ -457,9 +459,9 @@ public class ActionManager implements ActionController {
     /** {@inheritDoc} */
     @Override
     public ActionGroup createGroup(final String group) {
-        Logger.assertTrue(group != null);
-        Logger.assertTrue(!group.isEmpty());
-        Logger.assertTrue(!groups.containsKey(group));
+        checkNotNull(group);
+        checkArgument(!group.isEmpty());
+        checkArgument(!groups.containsKey(group));
 
         final File file = new File(getDirectory() + group);
         if (file.isDirectory() || file.mkdir()) {
@@ -475,9 +477,9 @@ public class ActionManager implements ActionController {
     /** {@inheritDoc} */
     @Override
     public void deleteGroup(final String group) {
-        Logger.assertTrue(group != null);
-        Logger.assertTrue(!group.isEmpty());
-        Logger.assertTrue(groups.containsKey(group));
+        checkNotNull(group);
+        checkArgument(!group.isEmpty());
+        checkArgument(groups.containsKey(group));
 
         for (Action action : groups.get(group).getActions()) {
             removeAction(action);
@@ -507,13 +509,13 @@ public class ActionManager implements ActionController {
     /** {@inheritDoc} */
     @Override
     public void changeGroupName(final String oldName, final String newName) {
-        Logger.assertTrue(oldName != null);
-        Logger.assertTrue(!oldName.isEmpty());
-        Logger.assertTrue(newName != null);
-        Logger.assertTrue(!newName.isEmpty());
-        Logger.assertTrue(groups.containsKey(oldName));
-        Logger.assertTrue(!groups.containsKey(newName));
-        Logger.assertTrue(!newName.equals(oldName));
+        checkNotNull(oldName);
+        checkArgument(!oldName.isEmpty());
+        checkNotNull(newName);
+        checkArgument(!newName.isEmpty());
+        checkArgument(groups.containsKey(oldName));
+        checkArgument(!groups.containsKey(newName));
+        checkArgument(!newName.equals(oldName));
 
         createGroup(newName);
 
@@ -545,7 +547,7 @@ public class ActionManager implements ActionController {
     /** {@inheritDoc} */
     @Override
     public List<ActionType> findCompatibleTypes(final ActionType type) {
-        Logger.assertTrue(type != null);
+        checkNotNull(type);
 
         final List<ActionType> res = new ArrayList<>();
         for (ActionType target : types) {
@@ -560,7 +562,7 @@ public class ActionManager implements ActionController {
     /** {@inheritDoc} */
     @Override
     public List<ActionComponent> findCompatibleComponents(final Class<?> target) {
-        Logger.assertTrue(target != null);
+        checkNotNull(target);
 
         final List<ActionComponent> res = new ArrayList<>();
         for (ActionComponent subject : components) {
@@ -575,7 +577,7 @@ public class ActionManager implements ActionController {
     /** {@inheritDoc} */
     @Override
     public List<ActionComparison> findCompatibleComparisons(final Class<?> target) {
-        Logger.assertTrue(target != null);
+        checkNotNull(target);
 
         final List<ActionComparison> res = new ArrayList<>();
         for (ActionComparison subject : comparisons) {
@@ -590,8 +592,8 @@ public class ActionManager implements ActionController {
     /** {@inheritDoc} */
     @Override
     public ActionComponent getComponent(final String type) {
-        Logger.assertTrue(type != null);
-        Logger.assertTrue(!type.isEmpty());
+        checkNotNull(type);
+        checkArgument(!type.isEmpty());
 
         for (ActionComponent target : components) {
             if (target.name().equals(type)) {
@@ -605,8 +607,8 @@ public class ActionManager implements ActionController {
     /** {@inheritDoc} */
     @Override
     public ActionComparison getComparison(final String type) {
-        Logger.assertTrue(type != null);
-        Logger.assertTrue(!type.isEmpty());
+        checkNotNull(type);
+        checkArgument(!type.isEmpty());
 
         for (ActionComparison target : comparisons) {
             if (target.name().equals(type)) {

src/com/dmdirc/commandparser/CommandArguments.java

@@ -24,13 +24,14 @@ package com.dmdirc.commandparser;
 
 import com.dmdirc.Precondition;
 import com.dmdirc.interfaces.CommandController;
-import com.dmdirc.logger.Logger;
 
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import static com.google.common.base.Preconditions.checkPositionIndex;
+
 /**
  * Represents a command and its arguments. In this class, input is split into
  * 'words' which are separated by any number of whitespace characters;
@@ -156,7 +157,7 @@ public class CommandArguments {
     public String getArgumentsAsString(final int start) {
         parse();
 
-        return getArgumentsAsString(start, words.length - 2);
+        return getArgumentsAsString(start, Math.max(start, words.length - 2));
     }
 
     /**
@@ -195,7 +196,7 @@ public class CommandArguments {
      */
     @Precondition("Start index is less than or equal to end index")
     public String getWordsAsString(final int start, final int end) {
-        Logger.assertTrue(start <= end + 1);
+        checkPositionIndex(start, end);
 
         final Pattern pattern = Pattern.compile("(\\S+\\s+){" + start + "}"
                 + "((\\S+\\s+){" + Math.max(0, end - start) + "}\\S+(\\s+$)?).*?");

src/com/dmdirc/config/IdentityManager.java

@@ -50,6 +50,9 @@ import java.util.Set;
 
 import lombok.extern.slf4j.Slf4j;
 
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+
 /**
  * The identity manager manages all known identities, providing easy methods
  * to access them.
@@ -268,8 +271,8 @@ public class IdentityManager implements IdentityFactory, IdentityController {
         "The specified File is a directory"
     })
     private void loadUser(final File dir) {
-        Logger.assertTrue(dir != null);
-        Logger.assertTrue(dir.isDirectory());
+        checkNotNull(dir);
+        checkArgument(dir.isDirectory());
 
         if (dir.listFiles() == null) {
             Logger.userError(ErrorLevel.MEDIUM,
@@ -421,7 +424,7 @@ public class IdentityManager implements IdentityFactory, IdentityController {
     /** {@inheritDoc} */
     @Override
     public void addConfigProvider(final ConfigProvider identity) {
-        Logger.assertTrue(identity != null);
+        checkNotNull(identity);
 
         final String target = getGroup(identity);
 
@@ -446,11 +449,11 @@ public class IdentityManager implements IdentityFactory, IdentityController {
     /** {@inheritDoc} */
     @Override
     public void removeConfigProvider(final ConfigProvider identity) {
-        Logger.assertTrue(identity != null);
+        checkNotNull(identity);
 
         final String group = getGroup(identity);
 
-        Logger.assertTrue(identities.containsValue(group, identity));
+        checkArgument(identities.containsValue(group, identity));
 
         synchronized (identities) {
             identities.remove(group, identity);
@@ -478,7 +481,7 @@ public class IdentityManager implements IdentityFactory, IdentityController {
     /** {@inheritDoc} */
     @Override
     public void registerIdentityListener(final String type, final ConfigProviderListener listener) {
-        Logger.assertTrue(listener != null);
+        checkNotNull(listener);
 
         synchronized (listeners) {
             listeners.add(type, listener);

src/com/dmdirc/logger/Logger.java

@@ -82,18 +82,6 @@ public final class Logger {
         getErrorManager().addError(level, message, exception, true);
     }
 
-    /**
-     * Asserts that the specified value is true. If not, an AssertionError
-     * exception is thrown.
-     *
-     * @param value The value to be tested
-     */
-    public static void assertTrue(final boolean value) {
-        if (!value) {
-            throw new AssertionError();
-        }
-    }
-
     /**
      * Gets the manager that should be used to report errors.
      *

src/com/dmdirc/ui/WindowManager.java

@@ -27,7 +27,6 @@ import com.dmdirc.FrameContainer;
 import com.dmdirc.Precondition;
 import com.dmdirc.interfaces.FrameCloseListener;
 import com.dmdirc.interfaces.ui.FrameListener;
-import com.dmdirc.logger.Logger;
 import com.dmdirc.util.collections.ListenerList;
 
 import java.util.Collection;
@@ -38,6 +37,9 @@ import java.util.concurrent.CopyOnWriteArrayList;
 import javax.inject.Inject;
 import javax.inject.Singleton;
 
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+
 /**
  * The WindowManager maintains a list of all open windows, and their
  * parent/child relations.
@@ -68,7 +70,7 @@ public class WindowManager {
      */
     @Precondition("The specified FrameListener is not null")
     public void addListener(final FrameListener frameListener) {
-        Logger.assertTrue(frameListener != null);
+        checkNotNull(frameListener);
 
         listeners.add(FrameListener.class, frameListener);
     }
@@ -146,8 +148,8 @@ public class WindowManager {
         "The specified Window has not already been added"
     })
     public void addWindow(final FrameContainer window, final boolean focus) {
-        Logger.assertTrue(window != null);
-        Logger.assertTrue(!rootWindows.contains(window));
+        checkNotNull(window);
+        checkArgument(!rootWindows.contains(window));
 
         rootWindows.add(window);
 
@@ -184,10 +186,10 @@ public class WindowManager {
     })
     public void addWindow(final FrameContainer parent,
             final FrameContainer child, final boolean focus) {
-        Logger.assertTrue(parent != null);
-        Logger.assertTrue(isInHierarchy(parent));
-        Logger.assertTrue(child != null);
-        Logger.assertTrue(!isInHierarchy(child));
+        checkNotNull(parent);
+        checkArgument(isInHierarchy(parent));
+        checkNotNull(child);
+        checkArgument(!isInHierarchy(child));
 
         parent.addChild(child);
 
@@ -224,8 +226,8 @@ public class WindowManager {
         "The specified window is in the window hierarchy"
     })
     public void removeWindow(final FrameContainer window) {
-        Logger.assertTrue(isInHierarchy(window));
-        Logger.assertTrue(window != null);
+        checkNotNull(window != null);
+        checkArgument(isInHierarchy(window));
 
         for (FrameContainer child : window.getChildren()) {
             child.close();
@@ -252,7 +254,7 @@ public class WindowManager {
      */
     @Precondition("The specified window name is not null")
     public FrameContainer findCustomWindow(final String name) {
-        Logger.assertTrue(name != null);
+        checkNotNull(name);
 
         return findCustomWindow(rootWindows, name);
     }
@@ -270,10 +272,9 @@ public class WindowManager {
         "The specified parent window is not null",
         "The specified parent window has been added to the Window Manager"
     })
-    public FrameContainer findCustomWindow(final FrameContainer parent,
-            final String name) {
-        Logger.assertTrue(parent != null);
-        Logger.assertTrue(name != null);
+    public FrameContainer findCustomWindow(final FrameContainer parent, final String name) {
+        checkNotNull(parent);
+        checkNotNull(name);
 
         return findCustomWindow(parent.getChildren(), name);
     }