Correct usage of bitmask?

Question

heyhey, just have a question about bitmasks. I think I know now what they are and where they can be used. I want to store specific permissions like BUILD, BREAK and INTERACT and maybe more for specific groups. The code below should do this but Im not very sure whether this is the corrent "style".

The idea is to use the first 3 bits here to store the permissions of the first group, then use the next three bits for the second group and so on. So my question is now whether this is a good way or not or what would be better?

public class Test {
    private int permissions = 0;

    /**
     * The amount of permissions, currently: {@link #BREAK}, {@link #BUILD}, {@link #INTERACT}
     */
    private static final int PERMISSIONS = 3;
    /**
     * The different permissions
     */
    public static final int BUILD = 1, BREAK = 2, INTERACT = 4;
    /**
     * The different groups
     */
    public static final int ALLIANCE = 0, OUTSIDERS = 1;

    public void setPermissions(int permissions, int group)
    {
        this.permissions = permissions << group * PERMISSIONS;
    }

    public void addPermissions(int permission, int group)
    {
        setPermissions(this.permissions | permission, group);
    }

    public boolean hasPermission(int permission, int group)
    {
        return (permissions & permission << group * PERMISSIONS) == permission;
    }
}

EDIT: I want to use as little memory as possible because I'll need to store much data.

EDIT: I also need to store it in a sql database, but it should not make probs.

Solution 1

You knew this kind of answer had to appear sooner or later, so here it goes:

Although the usage of bitmasks is arguably the fastest and has the lowest memory consuption of all the alternative options, it's also very error prone to get right and it's mostly discouraged to use except in some very edge cases. It's a classic low-level tool. If done right, works wonders, if misused, can wreak havoc.

Therefore, the correct approach is to use higher-level abstractions for this, namely enums and EnumSets. The speed and memory consuption are comparable, although slightly worse, of course. In a general case, they're absolutely sufficient, though. There are many ways of how to do that based on your exact context and needs. One of the possibilities could be this:

public enum Permission {
    BUILD, BREAK, INTERACT;
}

public class Permissions {
    private final Set<Permission> alliance = EnumSet.noneOf(Permission.class);
    private final Set<Permission> outsiders = EnumSet.noneOf(Permission.class);

    public Set<Permission> alliance() {
        return alliance;
    }

    public Set<Permission> outsiders() {
        return outsiders;
    }
}

This alone enables you to do exactly the things you did, but with two differences:

1. Now it's type-safe and more foolproof, I think. No reinventing of the wheel needed.
2. It uses more memory. Not by much, because EnumSet this small is usually just a long.

---

---

EDIT to answer OP's comment on storing the EnumSet to a database:

Yes, this can be a concern since storing an int is infinitely easier. If you still considered sticking to using an EnumSet, then there are several possibilities from the top of my head:

1. Look at SO. People have tried to tackle the problem before.

2. Save the names of the values in the EnumSet:

   Permissions p = new Permissions();
   p.alliance().addAll(EnumSet.of(Permission.BREAK, Permission.BUILD));
   for (Permission permission : p.alliance()) {
       System.out.println(permission);
   }
   

   You can then easily rebuild the values:

   for (String word : stringsFromDtb) {
       p.alliance.add(Permission.valueOf(word));
   }
   

3. Save the ordinals. This is VERY dangerous as you can easily break it with changing the Permission enum. Also, any random number could be fed in to break this.

   Permissions p = new Permissions();
   p.alliance().addAll(EnumSet.of(Permission.BREAK, Permission.BUILD));
   for (Permission permission : p.alliance()) {
       System.out.println(permission.ordinal());
   }
   

   You can then easily rebuild the values:

   for (int ordinal : ordinalsFromDtb) {
       p.alliance.add(Permission.values()[ordinal]);
   }
   

4. Serialize the EnumSet the usual way and store the binary data directly or BASE64ed. Ehm.

---

EDIT after EDIT:

Ad. your remarks of making indexes for your enum values so that when you changed or reordered them in the future, it would still work. There is an easy way to do this with enums! It's basically the middle way between bitfields and enums, it retains the type safety and all enum features and still has the advantages of bitfields.

public enum Permission {
    /* I like to have binary literals in place of bit fields,
     * but any notation will work */
    BUILD   (0b0001),
    BREAK   (0b0010),
    INTERACT(0b0100);

    private final int index;

    private Permission(int index) {
        this.index = index;
    }

    public int index() {
        return index;
    }
}

You'll then save the indexes into your database and will only have to make sure the parsing from it is right. Also, in the future, it helps to only comment out (not delete) any unneeded enum values so they still remain visible for you and you won't take up it's index. Or just mark it as @Deprecated and you don't have to delete anything ;).

Solution 2

There are a few points where I think you made an error

public void setPermissions(int permissions, int group)
{
    this.permissions = permissions << group * PERMISSIONS;
}

This clears all permissions for other groups when setting the permissions for group. If you want to retain the permissions for other groups,

// clear permissions for `group`
this.permissions &= ~(0x7 << group * PERMISSIONS);
// set permissions for `group`
this.permissions |= permissions << group * PERMISSIONS;

that leaves the permissions for the other goups untouched.

public void addPermissions(int permission, int group)
{
    setPermissions(this.permissions | permission, group);
}

This mixes the permissions for all groups with the permissions you want to add for group. I think what you want there is

this.permissions |= permission << group * PERMISSIONS;

to just add permission for group without changing anything else.

public boolean hasPermission(int permission, int group)
{
    return (permissions & permission << group * PERMISSIONS) == permission;
}

You are shifting in the wrong direction here, if group > 0, the bitwise and will have no bits among the lowest three,

return ((permissions >> group *PERMISSIONS) & permission) == permission;

Comments

[maxammann]

hm k also throught about this way, so another question because I need to store the permissions in a sql database. So I would give every enum an id and store then the ids in a string in the database. Is there any better way?

[Gus]

+1 for EnumSet. Somehow it's escaped my notice all these years.

[Gus]

@p000ison, Store the permissions in a Reference Table, and use a Junction Table to assign permissions

[Petr Janeček]

+1! Also, this is why we don't use complex bitmasks in production anymore :). When got right, it works wonders, but sometimes the complexity is too much.

[maxammann]

@Slanec So k, I can not store it in binary format, because maybe a website will access this data too, also storing the names is maybe not the best because maybe I change the names of the enums. To prefent the risk I would define ids for the enum entries and use those. But im not 100% sure whether I really want to use a EnumSet or I may use the bitmask. The only problem I see with bitmasks is that if I extend the permission system it will break :/

[maxammann]

@Daniel Fischer oh yeah thanks, first time I really use this thx :P

[maxammann]

Maybe Ill just use a long and make enough space for further permissions

[Petr Janeček]

@p000ison I know I'm quite late for this, but I've found this class that does the storing and loading of EnumSets into/from long easy and error-resistant.

[maxammann]

@Slanec oh nice thx, maybe I just use this way and use ids for the enum entries so if I change the order of them it still works. Thx :P

[Petr Janeček]

@p000ison Edited the edit to adress this in the best enum-like way possible :). Good luck to you!