49

I maintain a large document archive and I often use bit fields to record the status of my documents during processing or when validating them. My legacy code simply uses static int constants such as:

static int DOCUMENT_STATUS_NO_STATE = 0
static int DOCUMENT_STATUS_OK = 1
static int DOCUMENT_STATUS_NO_TIF_FILE = 2
static int DOCUMENT_STATUS_NO_PDF_FILE = 4

This makes it pretty easy to indicate the state a document is in, by setting the appropriate flags. For example:

status = DOCUMENT_STATUS_NO_TIF_FILE | DOCUMENT_STATUS_NO_PDF_FILE;

Since the approach of using static constants is bad practice and because I would like to improve the code, I was looking to use Enums to achieve the same. There are a few requirements, one of them being the need to save the status into a database as a numeric type. So there is a need to transform the enumeration constants to a numeric value. Below is my first approach and I wonder if this is the correct way to go about this?

class DocumentStatus{

    public enum StatusFlag {

        DOCUMENT_STATUS_NOT_DEFINED(1<<0),
        DOCUMENT_STATUS_OK(1<<1), 
        DOCUMENT_STATUS_MISSING_TID_DIR(1<<2),
        DOCUMENT_STATUS_MISSING_TIF_FILE(1<<3),
        DOCUMENT_STATUS_MISSING_PDF_FILE(1<<4),
        DOCUMENT_STATUS_MISSING_OCR_FILE(1<<5),
        DOCUMENT_STATUS_PAGE_COUNT_TIF(1<<6),
        DOCUMENT_STATUS_PAGE_COUNT_PDF(1<<7),
        DOCUMENT_STATUS_UNAVAILABLE(1<<8);


        private final long statusFlagValue;

        StatusFlag(long statusFlagValue) {
            this.statusFlagValue = statusFlagValue;
        }

        public long getStatusFlagValue(){
            return statusFlagValue;
        } 

       }


    /**
     * Translates a numeric status code into a Set of StatusFlag enums
     * @param numeric statusValue 
     * @return EnumSet representing a documents status
     */
    public EnumSet<StatusFlag> getStatusFlags(long statusValue) {
        EnumSet statusFlags = EnumSet.noneOf(StatusFlag.class);
        StatusFlag.each { statusFlag -> 
            long flagValue = statusFlag.statusFlagValue
            if ( (flagValue&statusValue ) == flagValue ) {
               statusFlags.add(statusFlag);
            }
        }
        return statusFlags;
    }


    /**
     * Translates a set of StatusFlag enums into a numeric status code
     * @param Set if statusFlags
     * @return numeric representation of the document status 
     */
    public long getStatusValue(Set<StatusFlag> flags) {
        long value=0;
        flags.each { statusFlag -> 
            value|=statusFlag.getStatusFlagValue() 
        }
        return value;
    }

     public static void main(String[] args) {

        DocumentStatus ds = new DocumentStatus();
        Set statusFlags = EnumSet.of(
            StatusFlag.DOCUMENT_STATUS_OK,
            StatusFlag.DOCUMENT_STATUS_UNAVAILABLE);

        assert ds.getStatusValue( statusFlags )==258 // 0000.0001|0000.0010

        long numericStatusCode = 56;
        statusFlags = ds.getStatusFlags(numericStatusCode);

        assert !statusFlags.contains(StatusFlag.DOCUMENT_STATUS_OK);
        assert statusFlags.contains(StatusFlag.DOCUMENT_STATUS_MISSING_TIF_FILE);
        assert statusFlags.contains(StatusFlag.DOCUMENT_STATUS_MISSING_PDF_FILE);
        assert statusFlags.contains(StatusFlag.DOCUMENT_STATUS_MISSING_OCR_FILE);

    }

}
Dubas
  • 2,855
  • 1
  • 25
  • 37
soappatrol
  • 533
  • 2
  • 6
  • 6
  • Why can't you write: static int DOCUMENT_STATUS_NO_PDF_FILE = 1<<2; ? – Ira Baxter Mar 17 '11 at 23:26
  • Reformatted code; please revert if incorrect. – trashgod Mar 17 '11 at 23:51
  • 5
    See also [Item 32: Use EnumSet instead of bit fields](http://java.sun.com/docs/books/effective/). – trashgod Mar 17 '11 at 23:58
  • I would like to avoid static constants such as static int DOCUMENT_STATUS_NO_PDF_FILE = 1<<2. My understanding is that it is considered bad practice since Enums were introduced in JDK1.5. – soappatrol Mar 18 '11 at 00:00
  • 1
    @trashgod link seems dead. [here](http://dhruba.name/2008/12/31/effective-java-item-32-use-enumset-instead-of-bit-fields/) is another one. – artronics May 02 '16 at 21:30
  • 2
    @SeyedJalalHosseini: See also *Item 32: Use EnumSet instead of bit fields*, cited [here](http://stackoverflow.com/a/6067501/230513). – trashgod May 02 '16 at 22:10
  • @trashgod one exception as to why you might want to use `long` flags instead of EnumSet is that new `EnumSets` are put on the heap where as `long` are not. This really only matters for extremely hi performance code. – Adam Gent Jan 09 '20 at 17:27
  • @AdamGent: The author notes that for sixty-four or fewer elements, "`EnumSet` is represented with a single `long`." – trashgod Jan 09 '20 at 21:13
  • 1
    @trashgod Every time you compose an EnumSet you are creating an object that is put on the heap and not the stack `if (universe.length <= 64) return new RegularEnumSet<>(elementType, universe);`. While it's small object that uses a native it's still an object. This only matters if you are aiming for zero garbage and or gc blips. In high performance loops you would not want to call `EnumSet.of` and prefer longs... etc etc. But this is an extreme fringe case. – Adam Gent Feb 27 '20 at 15:07

5 Answers5

33

Instead of defining constructor parameters, you could simply use the internal ordinal() value to calculate this.

public enum StatusFlag {

    DOCUMENT_STATUS_NOT_DEFINED,
    DOCUMENT_STATUS_OK, 
    DOCUMENT_STATUS_MISSING_TID_DIR,
    DOCUMENT_STATUS_MISSING_TIF_FILE,
    DOCUMENT_STATUS_MISSING_PDF_FILE,
    DOCUMENT_STATUS_MISSING_OCR_FILE,
    DOCUMENT_STATUS_PAGE_COUNT_TIF,
    DOCUMENT_STATUS_PAGE_COUNT_PDF,
    DOCUMENT_STATUS_UNAVAILABLE;


    public long getStatusFlagValue(){
        return 1 << this.ordinal();
    } 

}

Please note that now you should abstain from reordering, inserting (other than at the end) or deleting entries, otherwise the flag values will change, and the meaning of your database contents will change.

sloth
  • 99,095
  • 21
  • 171
  • 219
Paŭlo Ebermann
  • 73,284
  • 20
  • 146
  • 210
  • Is there a way to do boolean logic on this without the ugly looking method: `StatusFlag.DOCUMENT_STATUS_NOT_DEFINED.getStatusFlagValue() | StatusFlag.DOCUMENT_STATUS_MISSING_TID_DIR.getStatusFlagValue()` – bcoughlan Nov 20 '12 at 16:16
  • Using my approach, this could be done as `StatusFlag.DOCUMENT_STATUS_NOT_DEFINED.flag | StatusFlag.DOCUMENT_STATUS_MISSING_TID_DIR.flag` With static imports, it could be `DOCUMENT_STATUS_NOT_DEFINED.flag | DOCUMENT_STATUS_MISSING_TID_DIR.flag` – ChrisBlom Dec 20 '12 at 21:16
  • 16
    Using an ordinal may lead to fragile code. Simply reorganizing the list of items in the enum would break things since he is persisting to a db. – broconne Mar 14 '13 at 12:30
  • Your enum could also contain static combine(), remove() and test() methods to create the enum. StatusFlag.combine(DOCUMENT_STATUS_OK, DOCUMENT_STATUS_...) You could also get around the ordering thing if you wanted to by optionally supplying specific values. – Bill K Jul 30 '13 at 19:43
  • 4
    My advice is to always treat `ordinal()` as transient and to never serialise its value. Perfectly fine for a runtime only solution but should be avoided if serialising to a database. – Brett Ryan Nov 15 '15 at 16:15
22

your approach is exactly the way to do it.

Ray Tayek
  • 9,841
  • 8
  • 50
  • 90
  • 1
    Storing extra data in the enum values is pointless, as `ordinal()` already does this. – OrangeDog Mar 21 '11 at 12:38
  • 26
    @OrangeDog, I disagree. `ordinal()` can change if you refactor your enum. Take into account that [ordinal](http://docs.oracle.com/javase/7/docs/api/java/lang/Enum.html#ordinal%28%29) means each element's position in the enumeration. However, if you store extra data you can control it by yourself. – Aritz Sep 04 '13 at 15:15
10

A slightly better way would be to store the result of 1 << this.ordinal() in a field when the enum values are constructed. This way, you don't have to provide each value manually, and the flag is only computed once.

 public enum StatusFlag {
  DOCUMENT_STATUS_NOT_DEFIND,
  DOCUMENT_STATUS_OK, 
  DOCUMENT_STATUS_MISSING_TID_DIR,
  DOCUMENT_STATUS_MISSING_TIF_FILE,
  DOCUMENT_STATUS_MISSING_PDF_FILE,
  DOCUMENT_STATUS_MISSING_OCR_FILE,
  DOCUMENT_STATUS_PAGE_COUNT_TIF,
  DOCUMENT_STATUS_PAGE_COUNT_PDF,
  DOCUMENT_STATUS_UNAVAILABLE;

  public final int flag;

  StatusFlag() { 
    this.flag = 1 << this.ordinal();
  } 
}
**Update:** This is an old answer from back when I did not have much Java experience. I no longer think my answer is valid, as this approach couples the value of the flag to the ordering or the enum values, which is bad: if the order is changed or enum values are removed, this will affect the flags of other enum values, which can have unforeseen consequences.

These days, I would use the approach used in the question (manually provide the value of the flag via a constructor parameter) as it is more maintainable:

public enum StatusFlag {

  DOCUMENT_STATUS_NOT_DEFINED(0),
  DOCUMENT_STATUS_OK(1), 
  DOCUMENT_STATUS_MISSING_TID_DIR(2),
  DOCUMENT_STATUS_MISSING_TIF_FILE(3),
  DOCUMENT_STATUS_MISSING_PDF_FILE(4),
  DOCUMENT_STATUS_MISSING_OCR_FILE(5),
  DOCUMENT_STATUS_PAGE_COUNT_TIF(6),
  DOCUMENT_STATUS_PAGE_COUNT_PDF(7),
  DOCUMENT_STATUS_UNAVAILABLE(8);

  public final int flag;

  StatusFlag(int id) { 
    this.flag = 1 << id;
  } 
}
ChrisBlom
  • 1,262
  • 12
  • 17
  • Why precompute and cache such a trivial operation as a bit shift, rather than provide a method to compute the shifted value on demand? – Edward Brey Jun 10 '14 at 19:22
  • 1
    It doesn't matter much really in this case. I prefer to put fixed data in final fields and derived data in methods, this way I know for sure that these fields won't change at runtime. – ChrisBlom Jun 11 '14 at 07:32
  • In this case, the shifted value is both fixed and derived. Caching it provides a time vs. space tradeoff (unless the cache and allocation consequences of the extra space undoes any speed advantage). – Edward Brey Jun 11 '14 at 13:54
6

Don't give your enums values. Use an EnumSet to combine them, and use Enum.ordinal() when persisting in order to convert to/from a single integer. You might also find Class.getEnumConstants() useful when reconstructing the set from the integer.

OrangeDog
  • 36,653
  • 12
  • 122
  • 207
4

I have made a complete library for this problem: http://claude-martin.ch/enumbitset/

The main goal was to store sets of enum types in bitfields. But it also supports other types.

With this you would not need any extra methods like your "getStatusFlags()". It can be used on any existing enum type simply by adding the interface EnumBitSetHelper (it is used like a "trait"). Each enum constant can then create an "EnumBitSet" which has all methods of Java's EnumSet and BitSet. Then you can work with these sets of enum constants and convert them to bitfield values.

It supports many formats such as BigInteger and long to easily store the value into a bit field. But note that this only works with Java version 8 and newer.

Claude Martin
  • 745
  • 6
  • 21