Sunday, March 15, 2015

Dynamic properties in Java

Motivation

I've been looking at the problem of dynamic properties in Java. Typically properties are injected into a program at start and never change. Common examples include timeouts, host names, etc. Changing these requires and edit and restart (edit can also mean an update to a remote data source).

What I want is runtime updates to properties, and the program uses the new values. Hence "dynamic properties".

There are several schemes for this built around frameworks or external data sources. I want something using only the JDK.

Interfaces

My interfaces became one for tracking key-value pairs, one for updating them (javadoc munged to display in post):

/**
 * Tracking tracks string key-value pairs, tracking external updates
 * to the boxed values.  Optionally tracks them as a type converted from
 * string.  Example use: 
 * Tracking dynafig = ...;
 * Optional<AtomicReference<String>> prop = dynafig.track("prop");
 * boolean propDefined = prop.isPresent();
 * AtomicReference<String> propRef = prop.get();
 * String propValue = propRef.get();
 * // External source updates key-value pair for "prop"
 * String newPropValue = propRef.get();
* * In an injection context:
 * class Wheel {
 *     private final AtomicInteger rapidity;
 *
 *     @Inject
 *     public Wheel(final Tracking dynafig) {
 *         rapidity = dynafig.track("rapidity").
 *                 orElseThrow(() -> new IllegalStateException(
 *                         "Missing 'rapidity' property));
 *     }
 *
 *     public void spin() {
 *         spinAtRate(rapidity.get());
 *     }
 * }
* * @author Brian Oxley * @see Updating Updating key-value pairs * @see Default Reference implementation */ public interface Tracking { /** * Tracks the given key value as a string. Returns empty if * key is undefined. If key is defined, may stil * return a null boxed value. * * @param key the key, never missing * * @return the optional atomic value string, never missing * * @todo Some way to work out type from convert */ @Nonnull Optional<AtomicReference<String>> track(@Nonnull final String key); /** * Tracks the given key value as a boolean. Returns empty if * key is undefined. * * @param key the key, never missing * * @return the optional atomic value boolean, never missing */ @Nonnull Optional<AtomicBoolean> trackBool(@Nonnull final String key); /** * Tracks the given key value as an integer. Returns empty if * key is undefined. * * @param key the key, never missing * * @return the optional atomic value integer, never missing */ @Nonnull Optional<AtomicInteger> trackInt(@Nonnull final String key); /** * Tracks the given key value as type. Returns * empty if key is undefined. If key is defined, * may stil return a null boxed value. * * @param key the key, never missing * @param type the value type token, never missing * @param convert the value converter, never missing * @param <T> the value type * * @return the optional atomic value reference, never missing * * @todo Some way to work out type from convert */ @Nonnull <T> Optional<AtomicReference<T>> trackAs(@Nonnull final String key, @Nonnull final Class<T> type, // TODO: Can this be worked out? @Nonnull final Function<String, T> convert); }
/**
 * {@code Updating} updates key-value pairs.
 *
 * @author Brian Oxley
 * @see Tracking Tracking key-value pairs
 * @see Default Reference implementation
 */
public interface Updating {
    /**
     * Updates a key-value pair with a new value.
     *
     * @param key the key, never missing
     * @param value the value, possibly null
     *
     * @return true if updated else false if created
     */
    boolean update(@Nonnull final String key, @Nullable final String value);

    /**
     * Updates a key-value pair as a map entry for convenience.
     *
     * @param entry the entry, never missing
     *
     * @return true if updated else false if created
     * @see #update(String, String)
     */
    default boolean update(@Nonnull final Map.Entry<String, String> entry) {
        return update(entry.getKey(), entry.getValue());
    }

    /**
     * Updates a set of key-value pairs for convenience.  Each key is
     * invidually updated in entry-set order.
     *
     * @param values the key-value set, never missing
     *
     * @see #update(String, String)
     */
    default void updateAll(@Nonnull final Map<String, String> values) {
        values.entrySet().stream().
                forEach(this::update);
    }
}

Team lead

I built a reference implementation to demonstrate these were plausible. To my team I explained like this:

  • Coded very much in Java 8 functional style
  • Trivial API - 4 tracking calls (variants on return type), 1 updating call (plus 2 convenience)
  • Minimal dependencies - JDK + javax.annotations (@Nonnull)
  • Thread-safe, not concurrency-safe (lost updates, etc). Fine if properties are not updated on top of each other (i.e., several in the same microsecond)
  • I expect it to be straight-forward to integrate into JSR107, Cassandra, etc, but I've not tried this from home

And offered some advice to my juniors:

  • Be functional where sensible, the benefits are almost beyond enumeration
  • Corollary: Avoid state - in Java that means fields. Temp variables (locals) are debatable and come down to individual taste. I lean to avoiding them, but there's nothing wrong with creating locals that clarify the code for others (including yourself 6 mos. later)
  • Corollary: Push fields down as low as possible, avoid globals and "local globals" (static fields)
  • Complicate your data structure, not your calls. People reason about complex data structures much better than they do about complex logic
  • Avoid null. Really. Treat all uses of null as code smell - think of it as "machine level" programming. You should do high-level programming

Implementation

All tests pass. The reference implementation:

public final class Default
        implements Tracking, Updating {
    private final Map<String, Value> keys = new ConcurrentHashMap<>();
    private final Map<String, Value> values = new ConcurrentHashMap<>();

    public Default() {
    }

    public Default(@Nonnull final Map<String, String> keys) {
        updateAll(keys);
    }

    @Nonnull
    @Override
    public Optional<AtomicReference<String>> track(
            @Nonnull final String key) {
        return Optional.ofNullable(keys.get(key)).
                map(Value::get);
    }

    @Nonnull
    @Override
    public Optional<AtomicBoolean> trackBool(@Nonnull final String key) {
        return Optional.ofNullable(keys.get(key)).
                map(Value::getBool);
    }

    @Nonnull
    @Override
    public Optional<AtomicInteger> trackInt(@Nonnull final String key) {
        return Optional.ofNullable(keys.get(key)).
                map(Value::getInt);
    }

    @Nonnull
    @Override
    public <T> Optional<AtomicReference<T>> trackAs(@Nonnull final String key,
            @Nonnull final Class<T> type,
            @Nonnull final Function<String, T> convert) {
        return Optional.ofNullable(keys.get(key)).
                map(v -> v.getAs(type, convert));
    }

    @Override
    public boolean update(@Nonnull final String key,
            @Nullable final String value) {
        return null != keys.put(key, values.compute(key,
                (k, v) -> null == v ? new Value(value) : v.update(value)));
    }

    private static final class Atomic<T> {
        private final T atomic;
        private final Consumer<String> update;

        private static Atomic<AtomicReference<String>> of(
                final String value) {
            final AtomicReference<String> atomic = new AtomicReference<>(
                    value);
            return new Atomic<>(atomic, atomic::set);
        }

        private static Atomic<AtomicBoolean> boolOf(final String value) {
            final AtomicBoolean atomic = new AtomicBoolean(
                    null == value ? false : Boolean.valueOf(value));
            return new Atomic<>(atomic,
                    v -> atomic.set(null == v ? false : Boolean.valueOf(v)));
        }

        private static Atomic<AtomicInteger> intOf(final String value) {
            final AtomicInteger atomic = new AtomicInteger(
                    null == value ? 0 : Integer.valueOf(value));
            return new Atomic<>(atomic,
                    v -> atomic.set(null == v ? 0 : Integer.valueOf(v)));
        }

        private static <T> Atomic<AtomicReference<T>> asOf(final String value,
                final Function<String, T> convert) {
            final AtomicReference<T> atomic = new AtomicReference<>(
                    convert.apply(value));
            return new Atomic<>(atomic, v -> atomic.set(convert.apply(v)));
        }

        private Atomic(final T atomic, final Consumer<String> update) {
            this.atomic = atomic;
            this.update = update;
        }

        private void update(final String value) {
            update.accept(value);
        }
    }

    private final class Value {
        @Nullable
        private final String value;
        private final Map<Class<?>, Atomic<?>> values;

        private Value(final String value) {
            this(value, new ConcurrentHashMap<>(3));
        }

        private Value(@Nullable final String value,
                final Map<Class<?>, Atomic<?>> values) {
            this.value = value;
            this.values = values;
        }

        private Value update(final String value) {
            final Value newValue = new Value(value, values);
            newValue.values.values().stream().
                    forEach(a -> a.update(value));
            return newValue;
        }

        @SuppressWarnings("unchecked")
        private AtomicReference<String> get() {
            return (AtomicReference<String>) values.
                    computeIfAbsent(String.class,
                            k -> Atomic.of(value)).atomic;
        }

        private AtomicBoolean getBool() {
            return (AtomicBoolean) values.
                    computeIfAbsent(Boolean.class,
                            k -> Atomic.boolOf(value)).atomic;
        }

        private AtomicInteger getInt() {
            return (AtomicInteger) values.
                    computeIfAbsent(Integer.class,
                            k -> Atomic.intOf(value)).atomic;
        }

        @SuppressWarnings("unchecked")
        private <T> AtomicReference<T> getAs(final Class<T> type,
                final Function<String, T> convert) {
            return (AtomicReference<T>) values.
                    computeIfAbsent(type,
                            k -> Atomic.asOf(value, convert)).atomic;
        }

        @Override
        public boolean equals(final Object o) {
            if (this == o)
                return true;
            if (o == null || getClass() != o.getClass())
                return false;
            final Value that = (Value) o;
            return Objects.equals(value, that.value);
        }

        @Override
        public int hashCode() {
            return Objects.hash(value);
        }
    }
}

Wednesday, March 04, 2015

Does refactoring worsen code?

Thought provoking: Study finds that refactoring doesn’t improve code quality. The opening:

Refactoring software, that is, restructuring existing source code to make it more readable, efficient, and maintainable, is something all developers do every now and again. Of course, the implicit assumption behind refactoring is that the benefits (time and headaches saved in the future) outweigh the costs (time and effort spent now). However, new experimental research suggests that this may not be the case and that software code quality may not be improved much, if at all, by refactoring.

The study was done by researchers in Sri Lanka and recently published in the International Journal of Software Engineering & Applications titled An Empirical Evaluation of Impact of Refactoring On Internal and External Measures of Code Quality. The goal was to test whether common refactoring techniques resulted in measurable improvements in software quality, both externally (e.g., Is the code more maintainable?) and internally (e.g., Number of lines of code).

The researchers selected a small-scale application (about 4,500 lines of C# code) used by the academic staff at the University of Kelaniya for scheduling events and managing online documents for evaluation. 10 common refactoring techniques were applied to the code (e.g., Replace Type Code with Subclasses, Replace Conditional with Polymorphism).

Reading the press account I expect objections such as:

C#?
Well, language should not matter here
Too small an example!
This is a fair complaint, the impact of refactoring scales with code base size
Those weren't professionals
The reviewers were a mix, but this complaint overlooks how much code is written and maintained by "non-programmers", an oxymoron as programming is an activity, not a type of person

My own objection is that the refactoring did not go far enough. Deepening a type hierarchy does often make code "worse", delegation is often a better choice than inheritance. In a larger code base than the sample used, this become more obvious.