Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have a Value class that may represent many different types

private BigInteger fNum;
private BigDecimal fReal;
private ArrayList<StaticValue> fArrayValues;
private int fArrayOffset;
private HashMapArray<String, StaticValue> fRecordValues;
private boolean fIsCharLiteral;
private int fEnumOrd;
private char fCharLiteral;
private File fFile;
private StaticValue fLeft, fRight, fAscending;
private boolean fIsBuiltinBool = false;
private boolean fTruthValue;

You see it implements integer, real, vertor, enumeration and boolean. Some of the fields, fIsBuiltinBool and fIsCharLiteral hint which type it implements. Instances are created like

public StaticValue(ValueBuilder aBuilder) {
    super(aBuilder.getType());

    fId = aBuilder.getId();
    fIsCharLiteral = aBuilder.isCharLiteral();
    fCharLiteral = aBuilder.getCharLiteral();
    fEnumOrd = aBuilder.getEnumOrd();
    fNum = aBuilder.getNum();
    fFile = aBuilder.getFile();
    fReal = aBuilder.getReal();
    fLeft = aBuilder.getLeft();
    fRight = aBuilder.getRight();
    fAscending = aBuilder.getAscending();

    TypeStatic type = aBuilder.getType();

    switch (type.getCat()) {
    case ARRAY:
        fArrayOffset = init ...
        fArrayValues = init...
    case RECORD:
        fRecordValues = new HashMapArray<String, StaticValue>();
        init ...

    case INTEGER:
        if (fNum == null) throw Exception
            break;

and methods are implemented conditionally, like

public String toString() {

    if (fIsBuiltinBool) {
        return "" + fTruthValue;
    }

    try {
        Type type = getType();
        switch (type.getCat()) {
        case ARRAY: show fArrayValues 
        case RECORD:
            buf = new StringBuilder();
            show fields

and so on.

Despite promoters of OOP and design patterns state that OOP polymorphism is not any better than procedural case-based one, it looked very ugly to me. Additionally, I felt like polymorphism can eliminate the switch checking overhead and reduce memory footprint. I mean that I have millions of Value objects. When every instance allocates 10 fields, it takes some memory. Because fields are mutually exclusive, 9 of 10 fields are useless for every specific value type, we can save considerable memory. More compact object models better fit the cache, so performance must also improve. Improved cache efficiency will further improve performance in multithreaded application, where multiple processors compete for the computer bottleneck - the memory bus. The last motivation is debugging - when debugger displays the object fields, it is much easier to focus on substance when irrelevant fields are not shown.

This is how my polymorphic code looks like

public class StaticValue {

    protected StaticValue(ValueBuilder aBuilder) {
        type = aBuilder.getType();
        fId = aBuilder.getId();
    };

    public static class INT extends StaticValue {
        private BigInteger fNum;
        public INT(ValueBuilder aBuilder) {
            assert getType().isInteger();
            super(aBuilder);
            fNum = aBuilder.getNum();
            assert fNum != null;
        }
        public String toString() { return fNum.toString()}


    public static class REAL extends StaticValue {
        private BigDecimal fReal;

    public static class RECORD extends StaticValue {
        private List<StaticValue> fields;

I did not expect any serious performance gain since the project is very large and there are many other objects besides StaticValue. Yet, I still was surprised (experimental results are recorded interleaving between versions, so please do not think that OOP was recorded in another machine state, loaded by another process, and sorted):

Switches    OOP
32.62   34.1
30.32   33.15
30.9    31.9
30.7    31.755
30.9    31.5
30.14   31.45
30.2    31.3
33      31.2
32.4    31.137
30.4    31
30.364  30.36
30.7    29.82

Out of 30 seconds per test originally, new test finished in 31 seconds! It is a miniscule but new test takes longer! Why is the penalty? Why does the (JVM/Win/personal) machine prefer the conditional programs? Should I commit the changes into repository and continue refactoring the code into polymorphism?

share|improve this question
4  
Could you show a test too? –  palacsint Oct 28 '12 at 23:13
    
It is a set of integration tests of this very large project, zamiacad.sourceforge.net. Simulation tests run 30-33 seconds. So, 1 second is within the statistical uncertainty. However, I notice that conditional style fluctuations are generally 1 sec shorter. There is another test, that runs for 80 seconds and I get the same 1 second penalty. It is impossible to believe that this is a cost of loading 7 additional classes. I admit that polymorphism alone can run faster but, in the field, I am getting these results. –  Val Oct 29 '12 at 9:44
    
Why are people voting to close? This is relevant. –  codesparkle Nov 1 '12 at 10:49
    
does the tests allow for a warm up of the JIT/Hotspot Compiler? If not the values aren't really relevant. –  Jens Schauder Nov 2 '12 at 7:58
    
Of course they are irrelevant. Any data that shows something clearly and stably is irrelevant. Actually, it is your benchmark with heat up that is irrelevant. Eliminating heat up time from the benchmark deflects us away from what user will experience. We therefore also want to know whether OOP style favors the heat up time or degrades it ;) –  Val Nov 9 '12 at 15:40
show 1 more comment

5 Answers

Have you considered using Generic type attributes?

abstract class StaticValue<T> {
  T payload;
  //...
}

class Real extends StaticValue<BigDecimal> { /*...*/ }
share|improve this answer
    
It is a great idea. Though, it will not work if subtypes require type-specific behaviour and, secondly, does not answer my question, which is about performance. Otherwise, the idea is very good. –  Val Nov 15 '12 at 17:24
    
@Val Why wouldn't this approach work with type-specific behaviour? You just introduce "public abstract String doStuff();" method in the StaticValue<T> class, and then all the classes that extend it must implement their own, type-specific version of the public String doStuff() method. –  ZeroOne Nov 15 '12 at 23:38
    
You can add type specific behaviour using visitor pattern –  fixxer Nov 16 '12 at 5:36
add comment

There are many possible reasons. I'd look into the following things:

  • Memory footprint: You argued this should go down ... but you are creating extra objects so it might actually go up

  • Lifespan of your objects. Are your objects created and garbage collected often? This might cause a gc overhead with more objects even when the total memory consumption is less.

  • switch produces a pretty optimized byte code, the call to an overwritten method creates different byte code but has to include something similar to the switch as well. No idea which one is faster.

  • Make sure the JVM has enough cycles to warm up. Otherwise any performance benchmarks are pretty much useless.

share|improve this answer
    
Which extra objects I create? I just create appropriate slim object instead of fat ones. Why that should increase the lifetime? Why does warming up dislike OOP? –  Val Nov 2 '12 at 14:10
    
The 'more' objects was a misunderstanding on my side. Your code is different so the Hotspot compiler will behave different. That's all. Warming up doesn't dislike OOP. –  Jens Schauder Nov 4 '12 at 12:26
add comment

You might try making the StaticValue class be abstract, and have it include an abstract toString function. Doing this MAY reduce some internal JVM JiT compiler lookups... however that will depend on the particular JVM implementation and potentially the options you have set for it.

share|improve this answer
add comment

When you say your Value class can represent many data types, why don't you just use an Object class instead? That, too, can represent many data types. Then you could do something like this:

import java.math.BigDecimal;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

public class ConditionalsVsPolymorphism {

    // Here are your "Values", as Objects:
    private List values = new ArrayList();

    public static void main(String args[]) {
        ConditionalsVsPolymorphism instance = new ConditionalsVsPolymorphism();
        instance.run();
    }

    public ConditionalsVsPolymorphism() {
        Map<String, String> testMap = new HashMap<String, String>();
        testMap.put("stack", "overflow");
        testMap.put("code", "review");
        // Fill the array of values with some test data of different types:
        values.add(42);
        values.add("foobar");
        values.add(true);
        values.add(testMap);
        values.add(new BigDecimal(1234567L));
    }

    public void run() {
        // toString works just fine without any tricks:
        for (Object object : values) {
            System.out.println(object.getClass() + ": " + object);
        }

        // Casting the value of the object:
        for (Object object : values) {
            if (object instanceof Integer) {
                int myInt = Integer.class.cast(object);
                System.out.println("Cast to integer: " + myInt);
            } else if (object instanceof String) {
                String myString = String.class.cast(object);
                System.out.println("Cast to string: " + myString);
            } else if (object instanceof Boolean) {
                boolean myBool = Boolean.class.cast(object);
                System.out.println("Cast to boolean: " + myBool);
            } else if (object instanceof Map) {
                Map myMap = Map.class.cast(object);
                System.out.println("Cast to map: " + myMap);
            } else if (object instanceof BigDecimal) {
                BigDecimal myBigDecimal = BigDecimal.class.cast(object);
                System.out.println("Cast to BigDecimal: " + myBigDecimal);
            }
        }
    }
}

The code outputs:

class java.lang.Integer: 42
class java.lang.String: foobar
class java.lang.Boolean: true
class java.util.HashMap: {code=review, stack=overflow}
class java.math.BigDecimal: 1234567
Cast to integer: 42
Cast to string: foobar
Cast to boolean: true
Cast to map: {code=review, stack=overflow}
Cast to BigDecimal: 1234567

Alternatively, if you still do want to keep your Value class because it contains some logic, you can do it like this:

public class Value {

    private Object value;

    public Value(Object val) {
        value = val;
    }

    @Override
    public String toString() {
        if (isInteger()) {
            return "Integer: " + value;
        } else if (isString()) {
            return "String: " + value;
        } else {
            return value.toString();
        }
    }

    public boolean isInteger() {
        return value instanceof Integer;
    }

    public boolean isString() {
        return value instanceof String;
    }

    // A main method for testing this class:
    public static void main(String[] args) {
        Value val1 = new Value("Hello");
        Value val2 = new Value(1234);
        System.out.println("val1: " + val1);
        System.out.println("val2: " + val2);
    }
}

This code outputs:

val1: String: Hello
val2: Integer: 1234

Finally, if instanceof gives you some performance problems, you may replace object instanceof Integer with Integer.class.equals(object.getClass());. Note, however, that in that case a HashMap does not equal a Map, so you need to test for instantiable types.

Edit/Add: As to your final wondering "Why does the (JVM/Win/personal) machine prefer the conditional programs?": I guess we can only deduce that switches have been optimized better than polymorphism in the VM. The JVM7 Specification, for one, says that "Because of its emphasis on int comparisons, the Java virtual machine provides a rich complement of conditional branch instructions for type int." If you read that document more carefully, it might reveal you some interesting details of the differences between the two approaches. In general, I don't think that question is well suited to the Code Review site anyway, I believe you'd be better off posting that one to Programmers.

share|improve this answer
    
You ask, "why don't you just use an Object class instead?" I can tell you that it is not mine design. I just try to fix it by converting into OOP. Ok? Now, can you tell me how your question answers mine? –  Val Nov 14 '12 at 12:39
    
@Val Just read my other solution, starting with the sentence "Alternatively, if you still do want to keep your Value class because it contains some logic, you can do it like this". :) –  ZeroOne Nov 14 '12 at 21:05
    
Your "Alternative" is exactly the code that I want to convert into OOP. To understand that your answer has nothing to do with mine question, you must first look what the "replace Conditional with Polymorphism" means! –  Val Nov 15 '12 at 17:15
    
@Val Yeah, I get that, but what if replacing conditionals with polymorphism isn't the optimal way to go here? You say your main issues with the current way are that it a) is ugly, b) allocates too much memory and c) is hard to debug. I think the approach I have presented in my answer tackles all those problems. –  ZeroOne Nov 15 '12 at 23:35
    
@Val Also, please see my answer again, I have added a note to the end about the performance. –  ZeroOne Nov 16 '12 at 0:07
add comment

In a first glance, having StaticValueTypeXXX for each type and puting them in a Obervable/Observer pattern puts the ifTypeIs(..) in the Observers, not in a huge switch in Observable.

So you can reduce size of Observers [each toString() have small job to do with no test]
and hope for multithreading when notifyObservers() is activated.

Edit after comment

If the aim is to remove the switch, and with this, to have classes of different types with same named method loosely coupled (in place of one class multi-type, with polymorphism and a single method name),
Observalbe/Observer is powerful (and speed, and maintenable) in many situations I've experimented when I had intricated if or switch on object's type (not on values), even if it not the the main usage of this pattern.

share|improve this answer
    
wait, Observable is related with event listening. What has it to do with the OOP replacing type switch -> polymorphism? What do I have to listen? –  Val Oct 29 '12 at 12:50
    
@Val response in Edit –  cl-r Oct 29 '12 at 15:08
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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