Copy constructor help, trying to copy a boolean array. Java

Question

I have looked through as many previous questions as possible but never saw a question that had a boolean array as a variable.

Here is my class:

public class Register {

private boolean[] register;
private int length;

    //Normal constructor
public Register(int n) {

    if (n == 8 || n == 16 || n == 32 || n == 64) {

        length = n;
        register = new boolean[length];

        for (int i = 0; i < length; i++) {

            register[i] = false;
        }

    } else {

        throw new RegisterException(
                "A register can only contain 8, 16, 32, or 64 bits");
    }

}

// Creates a copy of reg (an existing Register)
public Register(Register reg) {

    length = reg.length;
    register = new boolean[reg.register.length];

    System.arraycopy(reg.register, 0, this.register, 0, reg.register.length);
}

In my driver program i am loading "1101101" into register1, but when i do: Register register2 = new Register(register1);

and print out both results i get:

0000000001101101

0000000000010110

Not really sure what is going on O.o any help would be appreciated, thanks!

This is my load method. i held off on putting it in here because it might be hard to read:

public void load(String binaryRep) {

    String allTheBits = binaryRep;
    int charPosition = 0;
    int loadLength;
    int binaryNum = 0;
    String index = "";
    String trimmedIndex = "";

    if (allTheBits.length() > 0 && allTheBits.length() <= length) {

        loadLength = allTheBits.length();

        for (int i = length - (loadLength); i < length; i++) {

            index = allTheBits.charAt(charPosition) + "";
            trimmedIndex = index.trim();
            binaryNum = Integer.parseInt(trimmedIndex);

            if (binaryNum == 1) {

                register[i] = true;

            } else if (binaryNum == 0) {

                register[i] = false;
            }

            charPosition++;

        }
    } else {
        throw new RegisterException("You can only load 0 - " + length
                + "bits.");
    }
}

Answer 1

Here's a more idiomatic way of doing it (using the Cloneable interface):

public class Register implements Cloneable {

private boolean[] register;

public Register(boolean[] register) {

    int n = register.length;

    if (n == 8 || n == 16 || n == 32 || n == 64) {
        this.register = register;
    } else {

        throw new IllegalArgumentException(
                "A register can only contain 8, 16, 32, or 64 bits");
    }

}

@Override
public String toString() {

    StringBuilder builder = new StringBuilder();

    for ( boolean b : this.register ) {
        builder.append( b ? "1" : "0" );
    }

    return builder.toString();
}

public Register( int n ) {
    this( new boolean[n] );
}

public int getLength() {
    return this.register.length;
}

@Override
public Register clone() {

    boolean[] clonedRegister = new boolean[this.register.length];

    System.arraycopy(this.register, 0, clonedRegister,0, this.register.length);

    return new Register( clonedRegister );
}

}

And a JUnit test showing it in action:

import org.junit.Assert;
import org.junit.Test;


public class RegisterTest {

    @Test
    public void testRegisterToString() {

        Register source = new Register( new boolean[] {true, true, false, false, true, false, true, false } );

        String result = "11001010";

        Assert.assertEquals( result, source.toString() );

    }

    @Test
    public void testRegisterCloning() {

        Register source = new Register( new boolean[] {true, true, false, false, true, false, false, false } );
        Register clone = source.clone();

        Assert.assertEquals( source.toString(), clone.toString() );

    }

}

Answer 2

A couple of remarks so that you learn some basic things.

1. As @Ted said, no need to keep the length field as register. length will give you the same
2. Local variables are not initialized with default values, but arrays are, as they are stored in the heap. So there's no need to iterate over the "register" array to set all of its positions to false
3. Using an array of booleans to do this may have felt easy but its extremely inefficient memory wise, as each boolean takes at least 32 bits in the heap. Therefore, to represent a 64 bit register you are using at least 32*64+32=2080 bits... using a byte array and bitwise logic will be a bit harder but hey, it's a small challenge :)

Anyway, your code looks fine (BTW, use Arrays.copyOf method as it's more readable), so the error should be coming from another side.

Answer 3

I just verified your load method with the following:

public static void main(String [] args)
{
    Register r1 = new Register(8);
    r1.load("1101101");
    Register r2 = new Register(r1);
    for (int i=0; i<8; i++) System.out.println(r2.register[i]);
}

Output:

> run Register
false
true
true
false
true
true
false
true
> 

It looks right to me as far as the contents of the Register objects are concerned, so the problem probably is with the access.