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.

Answer 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 ;).

Answer 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;