HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Boolean flags encoded as integer implemented with EnumSet

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
booleanenumsetwithimplementedencodedflagsinteger

Problem

I'm a beginner in Java so I would appreciate a review of following simple class - in fact it's my first real usage of enums.

The background: I'm parsing MySQL internal client-server protocol. One of the fields in server greeting and client authorization packet is "Client capabilities" which is 32 bit integer encoding 18 boolean flags ( if anyone is interested: http://forge.mysql.com/wiki/MySQL_Internals_ClientServer_Protocol#Handshake_Initialization_Packet)

I've read that the most 'java way' to implement such thing will be using EnumSet class.

I'll welcome any suggestions and other solutions - my needs are:

  • I must be able to parse flags from 4 bytes big endian buffer



  • To encode flags back to 4 bytes big endian buffer



  • To check if some arbitrary flag is set



here's the code:

```
public class ClientCapabilities
{
public enum Flag {
CLIENT_LONG_PASSWORD (1 flags = EnumSet.noneOf(Flag.class);

public ClientCapabilities(int capabilities)
{
originalInt = capabilities;
for (Flag f : Flag.values()) {
if ((capabilities & f.weight) > 0) {
flags.add(f);
}
}
}

public int getAsInt()
{
return originalInt;
}

public EnumSet getFlags()
{
return flags;
}

@Override
public String toString() {
return flags.toString();
}

//only tests ahead - TestNG, not JUnit!
@Test
public static void testDecodingClientCapabilities()
{
byte[] exampleBuffer = new byte[] { (byte) 0x85, (byte) 0xa6, (byte) 0x03, (byte) 0x00 };
EnumSet expectedEnumSet = EnumSet.of(
Flag.CLIENT_LONG_PASSWORD,
Flag.CLIENT_LONG_FLAG,
Flag.CLIENT_LOCAL_FILES,
Flag.CLIENT_PROTOCOL_41,
Flag.CLIENT_INTERACTIVE,
Flag.CLIENT_TRANSACTIONS,
Flag.CLIENT_SECURE_CONNECTION,
Flag.CLIENT_MULTI_STATEMENTS,

Solution

It looks fine. Some notes about the tests:

Test methods should be in another class (ClientCapabilitiesTest) and in a separate source tree. For example, Maven projects usually store sources under src/main/java and test sources under src/test/java. (Introduction to the Standard Directory Layout).

The first parameter of assertEquals is the expected value and the second is the actual one, so you should change

assertEquals(targetBuffer[1], (byte) 0x85);


to

assertEquals((byte) 0x85, targetBuffer[1]);


JUnit error messages assumes this order.

Create more tests which helps defect localization. A parameterized test class would be fine:

import static org.junit.Assert.assertEquals;

import java.util.Arrays;
import java.util.Collection;
import java.util.EnumSet;

import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;

@RunWith(Parameterized.class)
public class ClientCapabilitiesTest {

    private final byte[] input;
    private final EnumSet expected;

    @Parameters
    public static Collection data() {
        // TODO: add more cases
        final Object[][] data = new Object[][] {
                { new byte[] { (byte) 0x01, (byte) 0x00, (byte) 0x00, (byte) 0x00 },
                        EnumSet.of(Flag.CLIENT_LONG_PASSWORD) },
                { new byte[] { (byte) 0x02, (byte) 0x00, (byte) 0x00, (byte) 0x00 }, 
                        EnumSet.of(Flag.CLIENT_FOUND_ROWS) },
                { new byte[] { (byte) 0x03, (byte) 0x00, (byte) 0x00, (byte) 0x00 },
                        EnumSet.of(Flag.CLIENT_LONG_PASSWORD, Flag.CLIENT_FOUND_ROWS) },
                { new byte[] { (byte) 0x85, (byte) 0xa6, (byte) 0x03, (byte) 0x00 },
                        EnumSet.of(Flag.CLIENT_LONG_PASSWORD, 
                                Flag.CLIENT_LONG_FLAG, 
                                Flag.CLIENT_LOCAL_FILES,
                                Flag.CLIENT_PROTOCOL_41, 
                                Flag.CLIENT_INTERACTIVE, 
                                Flag.CLIENT_TRANSACTIONS,
                                Flag.CLIENT_SECURE_CONNECTION, 
                                Flag.CLIENT_MULTI_STATEMENTS, 
                                Flag.CLIENT_MULTI_RESULTS) } };
        return Arrays.asList(data);
    }

    public ClientCapabilitiesTest(final byte[] input, final EnumSet expected) {
        this.input = input;
        this.expected = expected;
    }

    @Test
    public void testDecodingClientCapabilities() {
        final ClientCapabilities capabilities = new ClientCapabilities(
            PacketByteBuffer.readIntFromFourBytes(input, 0));

        Assert.assertEquals(expected, capabilities.getFlags());
    }
}


I'd add one case for every flag and some cases with multiple enabled flags.

Code Snippets

assertEquals(targetBuffer[1], (byte) 0x85);
assertEquals((byte) 0x85, targetBuffer[1]);
import static org.junit.Assert.assertEquals;

import java.util.Arrays;
import java.util.Collection;
import java.util.EnumSet;

import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;

@RunWith(Parameterized.class)
public class ClientCapabilitiesTest {

    private final byte[] input;
    private final EnumSet<Flag> expected;

    @Parameters
    public static Collection<Object[]> data() {
        // TODO: add more cases
        final Object[][] data = new Object[][] {
                { new byte[] { (byte) 0x01, (byte) 0x00, (byte) 0x00, (byte) 0x00 },
                        EnumSet.of(Flag.CLIENT_LONG_PASSWORD) },
                { new byte[] { (byte) 0x02, (byte) 0x00, (byte) 0x00, (byte) 0x00 }, 
                        EnumSet.of(Flag.CLIENT_FOUND_ROWS) },
                { new byte[] { (byte) 0x03, (byte) 0x00, (byte) 0x00, (byte) 0x00 },
                        EnumSet.of(Flag.CLIENT_LONG_PASSWORD, Flag.CLIENT_FOUND_ROWS) },
                { new byte[] { (byte) 0x85, (byte) 0xa6, (byte) 0x03, (byte) 0x00 },
                        EnumSet.of(Flag.CLIENT_LONG_PASSWORD, 
                                Flag.CLIENT_LONG_FLAG, 
                                Flag.CLIENT_LOCAL_FILES,
                                Flag.CLIENT_PROTOCOL_41, 
                                Flag.CLIENT_INTERACTIVE, 
                                Flag.CLIENT_TRANSACTIONS,
                                Flag.CLIENT_SECURE_CONNECTION, 
                                Flag.CLIENT_MULTI_STATEMENTS, 
                                Flag.CLIENT_MULTI_RESULTS) } };
        return Arrays.asList(data);
    }

    public ClientCapabilitiesTest(final byte[] input, final EnumSet<Flag> expected) {
        this.input = input;
        this.expected = expected;
    }

    @Test
    public void testDecodingClientCapabilities() {
        final ClientCapabilities capabilities = new ClientCapabilities(
            PacketByteBuffer.readIntFromFourBytes(input, 0));

        Assert.assertEquals(expected, capabilities.getFlags());
    }
}

Context

StackExchange Code Review Q#7594, answer score: 3

Revisions (0)

No revisions yet.