3
\$\begingroup\$

This code checks value's type and cast it into appropriate type, then puts it into a javax.jcr.Node.

The checking looks long and boring. Is there a way to refactor it? I'm using Java 7.

/**
 * Set node's property.
 */
public static void setProperty(Node node, String propertyName, Object propertyValue) throws RepositoryException {
    if (node == null) {
        throw new IllegalArgumentException("Something wrong");
    }
    if (propertyName == null) {
        throw new IllegalArgumentException("Property name is invalid");
    }

    if (propertyValue == null) {
        node.setProperty(propertyName, (Value) null);
    } else if (propertyValue instanceof Value) {
        node.setProperty(propertyName, (Value) propertyValue);
    } else if (propertyValue instanceof Node) {
        node.setProperty(propertyName, (Node) propertyValue);
    } else if (propertyValue instanceof Binary) {
        node.setProperty(propertyName, (Binary) propertyValue);
    } else if (propertyValue instanceof Calendar) {
        node.setProperty(propertyName, (Calendar) propertyValue);
    } else if (propertyValue instanceof Date) {
        Calendar cal = Calendar.getInstance();
        cal.setTime((Date) propertyValue);
        node.setProperty(propertyName, cal);
    } else if (propertyValue instanceof BigDecimal) {
        node.setProperty(propertyName, (BigDecimal) propertyValue);
    } else if (propertyValue instanceof String) {
        node.setProperty(propertyName, (String) propertyValue);
    } else if (propertyValue instanceof Long) {
        node.setProperty(propertyName, ((Long) propertyValue).longValue());
    } else if (propertyValue instanceof Double) {
        node.setProperty(propertyName, (Double) propertyValue);
    } else if (propertyValue instanceof Boolean) {
        node.setProperty(propertyName, (Boolean) propertyValue);
    } else if (propertyValue instanceof InputStream) {
        node.setProperty(propertyName, inputStreamToBinary((InputStream) propertyValue));
    } else if (propertyValue instanceof Collection) {
        CollectionFactory factory = CollectionFactoryImpl.getInstance();
        ArrayList<Value> values = new ArrayList<Value>();
        for (Object value : (Collection<Object>) propertyValue) {
            values.add(createValue(value, factory));
        }
        node.setProperty(propertyName, values.toArray(new Value[values.size()]));
    } else {
        throw new IllegalArgumentException("Something wrong");
    }
}


@Test
public void test() {
    Node node = createNode("test");
    setProperty(node, "name", "name");
}
\$\endgroup\$
3
\$\begingroup\$

My idea would be to encapsulate the job of casting and converting into some kind of a Strategy pattern.

I would use an enum for that, namely NodeSettingStrategy:

public static enum NodeSettingValueStrategy {
    UNKNOWN(null) {
        @Override
        public void set(Node node, String propertyName, Object value) {
            throw new IllegalArgumentException("Cannot set property to a value of type " + value.getClass());
        }
    },

    DATE(Date.class) {
        @Override
        public void set(Node node, String propertyName, Object value) {
            node.setProperty(propertyName, (Long) value);
        }

    },

    LONG_PRIMITIVE(long.class) {
        @Override
        public void set(Node node, String propertyName, Object value) {
            node.setProperty(propertyName, (long) value);
        }
    },

    LONG(Long.class) {
        @Override
        public void set(Node node, String propertyName, Object value) {
            node.setProperty(propertyName, (Long) value);
        }
    },

    STRING(String.class) {
        @Override
        public void set(Node node, String propertyName, Object value) {
            node.setProperty(propertyName, (String) value);
        }
    },

    NODE(Node.class) {
        @Override
        public void set(Node node, String propertyName, Object value) {
            node.setProperty(propertyName, (Node) value);
        }
    };

    private final Class<?> supportedType;

    private NodeSettingValueStrategy(Class<?> supportedType) {
        this.supportedType = supportedType;
    }

    public static NodeSettingValueStrategy of(Class<?> type) {
        for (NodeSettingValueStrategy strategy : NodeSettingValueStrategy.values()) {
            if (strategy.supportedType == type) {
                return strategy;
            }

        }
        return NodeSettingValueStrategy.UNKNOWN;
    }

    public abstract void set(Node node, String propertyName, Object value);
}

Then I want to hide this NodeSettingStrategy by a thin abstraction, namely NodeSetter:

public static class NodeValueSetter {
    private final NodeSettingValueStrategy strategy;
    private final String propertyName;
    private final Object value;

    private NodeValueSetter(NodeSettingValueStrategy strategy, String propertyName, Object value) {
        this.strategy = strategy;
        this.propertyName = propertyName;
        this.value = value;
    }

    public void setTo(Node node) {
        strategy.set(node, propertyName, value);
    }

    public static NodeValueSetter of(final String propertyName, final Object value) {
        NodeSettingValueStrategy settingStrategy = NodeSettingValueStrategy.of(value.getClass());
        return new NodeValueSetter(settingStrategy, propertyName, value);
    }

}

Combine them together:

@Test
public void should_able_to_set_property() throws Exception {
        Node node = new Node();

        Node givenNode = new Node();
        String someString = "The brown fox jumps over a lazy dog";
        long givenLong = 234234234234L;

        for (NodeValueSetter setter : Arrays.asList(
                NodeValueSetter.of("a-string", someString),
                NodeValueSetter.of("a-number", givenLong),
                NodeValueSetter.of("a-node", givenNode))) {
            setter.setTo(node);
        }

        assertThat(node.getProperty("a-string"), is(someString));
        assertThat(node.getProperty("a-number"), is(givenLong));
        assertThat(node.getProperty("a-node"), is(givenNode));
    }
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.