2
\$\begingroup\$

The objective of the two classes below is to manage a collection of records where the record structure (i.e. the fields that comprise the record) is variable and defined at run-time. The record structure may contain integers, amounts, dates and strings.

The RecordDefinition class is used to define the record structure. For example, the class below defines a record that consists of an integer in field 1, and a string in field 2.

    RecordDefinition def = new RecordDefinition();
    def.defineField(RecordDefinition.TYPE_INT, 1);
    def.defineField(RecordDefinition.TYPE_STRING, 2);

When a record is created, it is based on a record definition. For example two records are created below. Note that the interface is similar to the one implemented by JDBC to set and get values depending on the type and position:

    Record r1 = new Record(def);
    r1.setInt(1, 1000);  // sets value in position 1
    r1.setString(2, "Some String"); // sets value in position 2

    Record r2 = new Record(def);
    r2.setInt(1, 2000);
    r2.setString(2, "Another String");

Then records can be added to collections, for example a list:

    List<Record> list = new ArrayList<>();      
    list.add(r1);
    list.add(r2);

To implement this, each Record class contains an array of each field type (i.e. an array of ints, and array of strings, etc). The arrays will contain the values (primitives are used when possible to avoid autoboxing). Also, arrays are used instead of ArrayList objects to improve performance.

A bi-directional HashMap will store the relationships between the field position in the record and the field position in the array (and vice versa). I'm using the Google Guava bi-directional HashMap for this purpose.

Am I on the right track or completely off charts? Any views on improving my approach (or changing it altogether!) will be greatly appreciated. This is the complete code (validation and exception handling removed for clarity):

import java.util.HashMap;
import java.util.Map;
import com.google.common.collect.BiMap;
import com.google.common.collect.HashBiMap;

public class RecordDefinition {

    public static final int TYPE_INT = 0;
    public static final int TYPE_AMOUNT = 1;
    public static final int TYPE_DATE = 2;
    public static final int TYPE_STRING = 3;

    public static final int NUMBER_OF_TYPES = 4;

    /*
     * Each entry determines the number of fields in the record for each type
     */
    private int [] numberOfFields = new int [NUMBER_OF_TYPES];

    /*
    * For each type, have a BiMap that determines a field position 
    * in the record given the position in the array (and vice versa)
    */
    private Map<Integer,BiMap<Integer,Integer>> positions = null;

    public RecordDefinition() {

        positions = new HashMap<>();

        // for each field type, create BiMap
        // and initialize number of fields
        for ( int i=0; i<NUMBER_OF_TYPES; i++ ) {
            BiMap<Integer, Integer> map = HashBiMap.create(); 
            positions.put(i, map);
            numberOfFields[i] = 0;
        }

    }

    /*
     * Defines the position of a field in the record
     * 
     * @param field type (int, amount, etc.)
     * @param field position in the record
     */
    public void defineField (int type, int positionInRecord) {
        BiMap<Integer, Integer> map = positions.get(type);
        map.put(positionInRecord, numberOfFields[type]);
        numberOfFields[type]++; 
    }


    public int getNumberOfFields (int type) {
        return numberOfFields[type]; 
    }


    // get position in array, given field type and position in record
    public int getPositionInArray(int type, int positionInRecord) {

        BiMap<Integer, Integer> map = positions.get(type);
        return map.get(positionInRecord);

    }


    // get position in record, given field type and position in array
    public int getPositionInRecord(int type, int positionInArray) {

        BiMap<Integer, Integer> map = positions.get(type);
        return map.inverse().get(positionInArray);
    }
}


import java.util.Date;

public class Record {

    private RecordDefinition definition = null;

    private double [] doubles = null;
    private Date [] dates = null;
    private int [] ints = null;
    private String [] strings = null;

    public Record (RecordDefinition definition) {

        this.definition = definition;

        int size = definition.getNumberOfFields(RecordDefinition.TYPE_INT); 
        if ( size > 0 )
            ints = new int[size]; 

        size = definition.getNumberOfFields(RecordDefinition.TYPE_AMOUNT); 
        if ( size > 0 )
            doubles = new double[size];

        size = definition.getNumberOfFields(RecordDefinition.TYPE_DATE); 
        if ( size > 0 )
            dates = new Date[size];

        size = definition.getNumberOfFields(RecordDefinition.TYPE_STRING); 
        if ( size > 0 )
            strings = new String[size];

    }


    public void setInt (int positionInRecord, int value ) {
        int positionInArray = definition.getPositionInArray(RecordDefinition.TYPE_INT, positionInRecord);
        ints [positionInArray] = value;
    }

    public int getInt (int positionInRecord) {
        int positionInArray = definition.getPositionInArray(RecordDefinition.TYPE_INT, positionInRecord);
        return ints [positionInArray];
    }



    public void setString (int positionInRecord, String value ) {
        int positionInArray = definition.getPositionInArray(RecordDefinition.TYPE_STRING, positionInRecord);
        strings [positionInArray] = value;
    }

    public String getString (int positionInRecord) {
        int positionInArray = definition.getPositionInArray(RecordDefinition.TYPE_STRING, positionInRecord);
        return strings [positionInArray];
    }


    /// Other getters and setters
}

Classes in github can be found here

\$\endgroup\$
5
  • \$\begingroup\$ Why not use a Builder pattern? Something like what is done in Dragon class here \$\endgroup\$
    – yadav_vi
    Commented Jan 15, 2015 at 17:39
  • \$\begingroup\$ The Builder class has predefined members; I need these to be defined at run-time. \$\endgroup\$
    – ps0604
    Commented Jan 15, 2015 at 17:43
  • \$\begingroup\$ I still am not convinced, because you have your types like RecordDefinition.TYPE_DATE defined here. \$\endgroup\$
    – yadav_vi
    Commented Jan 15, 2015 at 17:46
  • \$\begingroup\$ The thing is, yadav_vi, is that the record order and number isn't defined until instantiation based on the def object passed as an argument (i.e. runtime). I think the approach is logical, although I'm not familiar enough with Google Guava to comment on your implementation. \$\endgroup\$
    – gankoji
    Commented Jan 15, 2015 at 17:58
  • \$\begingroup\$ Guava is just the implementation of a bi-directional HashMap to get the field position in the record given the position in the array and vice versa \$\endgroup\$
    – ps0604
    Commented Jan 15, 2015 at 18:19

1 Answer 1

1
\$\begingroup\$

It may be preferable to remove the index from defineField() so that they're always stored in the order they're added. Letting clients pick their own indexes may be asking for trouble.

FieldType.INTEGER might be preferable to RecordDefinition.TYPE_INT. FieldType should, of course, be an enum.

You might also choose to have defineField() return this, for chaining.

Personal preference, I like addField() better, since there's really no defining unless you've hidden details.

final RecordDefinition recordDefinition = 
    new RecordDefinition()
        .addField(FieldType.INT)
        .addField(FieldType.STRING);

That may or may not look better to you. Another option, assuming you know you'll have a limited number of types forever, might be

final RecordDefinition recordDefinition = 
    new RecordDefinition()
        .addIntField()
        .addStringField();

You give up a little flexibility for a little readability. Regardless of what you choose, do consider pulling the indexes unless you need them. They can always be added later, and they make the definition instantiation look sloppy.

To implement this, each Record class contains an array of each field type (i.e. an array of ints, and array of strings, etc). The arrays will contain the values (primitives are used when possible to avoid autoboxing). Also, arrays are used instead of ArrayList objects to improve performance.

This sounds like premature optimization to me. Do you have an execution time problem where this is the demonstrated bottleneck? Presumably the record definition is fixed at construction time, and you're initializing the array sizes based on the definition, then doing some kind of mapping of field number to array location? Sounds messy and vulnerable to errors.

You've got three pieces of information - field index, field type, field value. The definition holds two pieces - field index and field type. The record will also need two pieces, field index and value. It can use its RecordDefinition to determine the type.

Are you sure you don't want record to just have an Object[] and do some casting to get values out? That would look something like:

RecordDefinition

import java.util.ArrayList;
import java.util.List;

public final class RecordDefinition {

    public enum FieldType {
        INT, AMOUNT, DATE, STRING
    };

    private final List<FieldType> fieldTypes = new ArrayList<FieldType>();

    public RecordDefinition() {
        super();
    }

    /* We need a copy constructor so that Record can be sure the definition doesn't
     * change after it gets passed in. Or you could use a builder pattern for
     * RecordDefinition so the Record gets an immutable instance. */
    RecordDefinition(final RecordDefinition recordDefinition) {
        this.fieldTypes.addAll(recordDefinition.fieldTypes);
    }

    public RecordDefinition addField(final FieldType fieldType) {
        if (fieldType == null) {
            throw new IllegalArgumentException("fieldType parameter may not be null.");
        }
        this.fieldTypes.add(fieldType);
        return this;
    }

    public int getNumberOfFields() {
        return this.fieldTypes.size();
    }

    public FieldType getFieldTypeAt(final int index) {
        return this.fieldTypes.get(index);
    }

}

Record

public final class Record {

    private final RecordDefinition definition;

    private final Object[] values;

    public Record(final RecordDefinition definition) {
        this.definition = new RecordDefinition(definition);
        this.values = new Object[this.definition.getNumberOfFields()];
    }

    public void setInt(final int index, final int value) {
        this.checkFieldType(index, RecordDefinition.FieldType.INT);
        this.values[index] = Integer.valueOf(value);
    }

    public int getInt(final int index) {
        this.checkFieldType(index, RecordDefinition.FieldType.INT);
        return ((Integer) this.values[index]).intValue(); // can also put the in the FieldType
    }

    private void checkFieldType(final int index, final RecordDefinition.FieldType fieldType) {
        if (!this.definition.getFieldTypeAt(index).equals(RecordDefinition.FieldType.INT)) {
            // UHOH! Throw an exception
        }
    }

}

Another option for Record would be to trade memory usage for runtime performance and readability by using sparse arrays:

public class Record {

    private final RecordDefinition definition;

    private final double[] doubles;
    private final Date[] dates;
    private final int[] ints;
    private final String[] strings;

    public Record(final RecordDefinition definition) {
        this.definition = new RecordDefinition(definition);
        this.doubles = new double[definition.getNumberOfFields()];
        this.dates = new Date[definition.getNumberOfFields()];
        this.ints = new int[definition.getNumberOfFields()];
        this.strings = new String[definition.getNumberOfFields()];
    }

    public void setInt(final int index, final int value) {
        this.checkFieldType(index, RecordDefinition.FieldType.INT);
        this.ints[index] = value;
    }

    public int getInt(final int index) {
        this.checkFieldType(index, RecordDefinition.FieldType.INT);
        return this.ints[index];
    }

    private void checkFieldType(final int index, final RecordDefinition.FieldType fieldType) {
        if (!this.definition.getFieldTypeAt(index).equals(RecordDefinition.FieldType.INT)) {
            // UHOH! Throw an exception
        }
    }
}

I think either of those is better (more readable/less bug prone) than trying to work out a mapping the way you are. And however you choose to proceed, I encourage you not to try to solve performance problems until you know you have them. The only way to optimize for both memory usage and run-time performance is to pay in code readability. Try not to do that unless you really, really need to!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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