» Refactoring Teaser III » Managed Chaos by Naresh Jain

Refactoring Teaser III

This time a simple one.

Following test will help you understand the code:

public class MyLoggerTest implements Console {
    private String msg;
    private MyLogger logger = new MyLogger(this);
 
    @Test
    public void handleNonIOExceptions() {
        logger.error(new IllegalArgumentException("Ignore Exception"));
        assertEquals("SEVERE: Dying due to exception : Ignore Exception", msg);
    }
 
    @Test
    public void ignoreSpecificIOExceptions() {
        String errorMsg = "Broken pipe:" + Math.random();
        logger.error(new IOException(errorMsg));
        assertEquals("FINE: Ignoring Exception for : " + errorMsg, msg);
    }
 
    @Test
    public void handleGenericIOExceptions() {
        String errorMsg = "Random IO Error:" + Math.random();
        logger.error(new IOException(errorMsg));
        assertEquals("SEVERE: Dying due to exception : " + errorMsg, msg);
    }
 
    @Override
    public void write(final String msg) {
        this.msg = msg;
    }
}

public class MyLogger {
    private final Console out;
 
    public MyLogger(final Console out) {
        this.out = out;
    }
 
    private static final String[] IGNORED_IOEXCEPTION_MESSAGES = {
            "An existing connection was forcibly closed by the remote host",
            "Connection reset by peer",
            "Broken pipe",
            "Connection timed out",
            "No route to host",
            };
 
    public void error(final Throwable t) {
        if (isIgnored(t)) {
            out.write("FINE: Ignoring Exception for : " + t.getMessage());
        } else {
            out.write("SEVERE: Dying due to exception : " + t.getMessage());
        }
    }
 
    private boolean isIgnored(final Throwable t) {
        if (t instanceof IOException) {
            final String exceptionMessage = t.getMessage();
            for (String ignoredMessage : IGNORED_IOEXCEPTION_MESSAGES) {
                if (exceptionMessage.startsWith(ignoredMessage)) {
                    return true;
                }
            }
        }
        return false;
    }
}

and

public interface Console {
    void write(String msg);
}

Feel free to download the Java project.

This entry was posted on Wednesday, August 5th, 2009 at 9:05 PM and is filed under Agile, Design, Programming, Testing. You can follow any responses to this entry through the RSS 2.0 feed. You can skip to the end and leave a response. Pinging is currently not allowed.

http://finalprefix.com/ Joel Rosario

This clojure code uses the mini test framework from my submission to your previous teaser. I really should shop around for an existing clojure test framework.

(import 'java.io.IOException)
 
(defn ignore? [exception]
  (if (let [exception-message (.getMessage exception)]
	   (some #(.startsWith exception-message %)
		 #{"An existing connection was forcibly closed by the remote host"
		 "Connection reset by peer"
		 "Broken pipe"
		 "Connection timed out"
		 "No route to host"}))
      :ignored
    :severe))
 
(defmulti log-error type)
(defmethod log-error :ignored [exception] (println "FINE: Ignoring exception for:" (exception :message)))
(defmethod log-error :severe [exception] (println "SEVERE: Dying due to exception:"  (exception :message)))
(defmethod log-error IOException [exception] (log-error (with-meta {:message (.getMessage exception)} {:type (ignore? exception)})))
(defmethod log-error Throwable [exception] (log-error (with-meta {:message (.getMessage exception)} {:type :severe})))
 
(use 'spec)
 
(tests
 (spec
  "Ignore specific IOExceptions"
  (given :error-msg (str "Broken pipe: " (Math/random)))
  (fn [error-msg] (.trim (with-out-str (log-error (IOException. error-msg)))))
  (should (fn [result] (.startsWith result "FINE: Ignoring exception for: Broken pipe: "))))
 
 (spec
  "Handle generic IOExceptions"
  (given :error-msg (str "Random IO Error: " + (Math/random)))
  (fn [error-msg] (.trim (with-out-str (log-error (IOException. error-msg)))))
  (should (fn [result] (.startsWith result "SEVERE: Dying due to exception: Random IO Error: "))))
 
 (spec
  "Handle non IOExceptions"
  (given :exception (Exception. "Ignore Exception"))
  (fn [exception] (.trim (with-out-str (log-error exception))))
  (returns "SEVERE: Dying due to exception: Ignore Exception")))

http://finalprefix.com Joel Rosario

This clojure code uses the mini test framework from my submission to your previous teaser. I really should shop around for an existing clojure test framework.

(import 'java.io.IOException)
 
(defn ignore? [exception]
  (if (let [exception-message (.getMessage exception)]
	   (some #(.startsWith exception-message %)
		 #{"An existing connection was forcibly closed by the remote host"
		 "Connection reset by peer"
		 "Broken pipe"
		 "Connection timed out"
		 "No route to host"}))
      :ignored
    :severe))
 
(defmulti log-error type)
(defmethod log-error :ignored [exception] (println "FINE: Ignoring exception for:" (exception :message)))
(defmethod log-error :severe [exception] (println "SEVERE: Dying due to exception:"  (exception :message)))
(defmethod log-error IOException [exception] (log-error (with-meta {:message (.getMessage exception)} {:type (ignore? exception)})))
(defmethod log-error Throwable [exception] (log-error (with-meta {:message (.getMessage exception)} {:type :severe})))
 
(use 'spec)
 
(tests
 (spec
  "Ignore specific IOExceptions"
  (given :error-msg (str "Broken pipe: " (Math/random)))
  (fn [error-msg] (.trim (with-out-str (log-error (IOException. error-msg)))))
  (should (fn [result] (.startsWith result "FINE: Ignoring exception for: Broken pipe: "))))
 
 (spec
  "Handle generic IOExceptions"
  (given :error-msg (str "Random IO Error: " + (Math/random)))
  (fn [error-msg] (.trim (with-out-str (log-error (IOException. error-msg)))))
  (should (fn [result] (.startsWith result "SEVERE: Dying due to exception: Random IO Error: "))))
 
 (spec
  "Handle non IOExceptions"
  (given :exception (Exception. "Ignore Exception"))
  (fn [exception] (.trim (with-out-str (log-error exception))))
  (returns "SEVERE: Dying due to exception: Ignore Exception")))

http://finalprefix.com/ Joel Rosario

This approach turns the error level into a strategy. So logging a message reads: (log error “big bad wolf”)

(ns agilefaqs.refactoring.teaser.3.take2
    (:import java.io.IOException))
 
(defn ignore? [exception]
  (let [exception-message (.getMessage exception)]
       (some #(.startsWith exception-message %)
	     #{"An existing connection was forcibly closed by the remote host"
	     "Connection reset by peer"
	     "Broken pipe"
	     "Connection timed out"
	     "No route to host"})))
 
(defn severe [exception]
  (println "SEVERE: Dying due to exception:"  (.getMessage exception)))
 
(defn fine [exception]
  (println "FINE: Ignoring exception for:" (.getMessage exception)))
 
(defmulti error type)
(defmethod error IOException [exception]
  (if (ignore? exception) fine severe))
(defmethod error Throwable [exception] severe)
 
(defn log [facility exception]
  (let [result (facility exception)]
       (when (ifn? result) (recur result exception))))
 
(use 'spec)
 
(tests
 (spec
  "Ignore specific IOExceptions"
  (given :error-msg (str "Broken pipe: " (Math/random)))
  (fn [error-msg] (.trim (with-out-str (log error (IOException. error-msg)))))
  (should (fn [result] (.startsWith result "FINE: Ignoring exception for: Broken pipe: "))))
 
 (spec
  "Handle generic IOExceptions"
  (given :error-msg (str "Random IO Error: " + (Math/random)))
  (fn [error-msg] (.trim (with-out-str (log error (IOException. error-msg)))))
  (should (fn [result] (.startsWith result "SEVERE: Dying due to exception: Random IO Error: "))))
 
 (spec
  "Handle non IOExceptions"
  (given :exception (Exception. "Ignore Exception"))
  (fn [exception] (.trim (with-out-str (log error exception))))
  (returns "SEVERE: Dying due to exception: Ignore Exception")))

http://finalprefix.com Joel Rosario

This approach turns the error level into a strategy. So logging a message reads: (log error “big bad wolf”)

(ns agilefaqs.refactoring.teaser.3.take2
    (:import java.io.IOException))
 
(defn ignore? [exception]
  (let [exception-message (.getMessage exception)]
       (some #(.startsWith exception-message %)
	     #{"An existing connection was forcibly closed by the remote host"
	     "Connection reset by peer"
	     "Broken pipe"
	     "Connection timed out"
	     "No route to host"})))
 
(defn severe [exception]
  (println "SEVERE: Dying due to exception:"  (.getMessage exception)))
 
(defn fine [exception]
  (println "FINE: Ignoring exception for:" (.getMessage exception)))
 
(defmulti error type)
(defmethod error IOException [exception]
  (if (ignore? exception) fine severe))
(defmethod error Throwable [exception] severe)
 
(defn log [facility exception]
  (let [result (facility exception)]
       (when (ifn? result) (recur result exception))))
 
(use 'spec)
 
(tests
 (spec
  "Ignore specific IOExceptions"
  (given :error-msg (str "Broken pipe: " (Math/random)))
  (fn [error-msg] (.trim (with-out-str (log error (IOException. error-msg)))))
  (should (fn [result] (.startsWith result "FINE: Ignoring exception for: Broken pipe: "))))
 
 (spec
  "Handle generic IOExceptions"
  (given :error-msg (str "Random IO Error: " + (Math/random)))
  (fn [error-msg] (.trim (with-out-str (log error (IOException. error-msg)))))
  (should (fn [result] (.startsWith result "SEVERE: Dying due to exception: Random IO Error: "))))
 
 (spec
  "Handle non IOExceptions"
  (given :exception (Exception. "Ignore Exception"))
  (fn [exception] (.trim (with-out-str (log error exception))))
  (returns "SEVERE: Dying due to exception: Ignore Exception")))

http://agilefaqs.com/nareshjain.html Naresh Jain

This is a really verbose solution to this problem:

Defined a new class called ErrorMessage to handle Error Message Comparison.

private static class ErrorMessage {
 
    private final String msg;
 
    public ErrorMessage(final String msg) {
        this.msg = msg;
    }
 
    @Override
    public boolean equals(final Object other) {
        try {
            return msg.startsWith(((MyLogger.ErrorMessage) other).msg);
        } catch (Throwable t) {
            return false;
        }
    }
}

Also defined two methods, one which takes IOException and other takes Throwable. This helps us avoid the instanceOf check.

public void error(final Throwable t) {
    out.write("SEVERE: Dying due to exception : " + t.getMessage());
}
 
public void error(final IOException e) {
    if (IGNORED_ERROR_MESSAGES.contains(errorMessageOf(e))) {
        out.write("FINE: Ignoring Exception for : " + e.getMessage());
    } else {
        error((Throwable) e);
    }
}

Had to defined a list of Ignore error messages as:

private static final List IGNORED_ERROR_MESSAGES = new ArrayList() {
    {
        add(new ErrorMessage("An existing connection was forcibly closed by the remote host"));
        add(new ErrorMessage("Connection reset by peer"));
        add(new ErrorMessage("Broken pipe"));
        add(new ErrorMessage("Connection timed out"));
        add(new ErrorMessage("No route to host"));
    }
};

I’m not very happy with this solution, but it does separate the responsibilities at the cost of making the code more verbose.