Code Cleanup: Not Just About Refactoring Part 3

Table of Contents

Introduction

Due to formatting issues on the blogspot, it is advisable to read this article as a PDF file. You can download the PDF version of the article here.

Lazy Variable Declaration

Let’s examine the boolMultiply method and the local variable named sum. This variable is declared at the beginning of the method and is used much later. This habit comes from procedural languages (such as early C and PL/SQL), where you were required to declare all variables at the start of the method. Fortunately, most modern programming languages (especially object-oriented ones) no longer have this limitation. Instead, I suggest applying the principle of lazy declaration of variables, which can be summed up as:

Declare variables as late as possible.

Doing so makes it easier to read the code if the operations performed on a given variable are within the sightline. In more complex methods, placing the declaration at the beginning might cause confusion about whether a variable has been processed yet or what has affected its value.

Assume we scroll the screen to see the following code:

for (int i = 0; i < len; i++) {
    if (this.get(i) && extendedBitSet.get(i)) {
        sum++;
    }
}
return sum % 2;

An immediate question arises — what is this sum? Has anything been done with it earlier? Or was its value obtained externally?

When dealing with a single variable, it’s not a significant issue, but imagine having five such variables. Tracking the logic of such a method becomes very challenging.

A simple operation of moving the declaration a bit lower will make the code much more readable:

public int boolMultiply(ExtendedBitSet extendedBitSet) {
    int len = 0;

    if (this.fixedLength < extendedBitSet.fixedLength) {
        len = this.fixedLength;
    } else {
        len = extendedBitSet.fixedLength;
    }

    int sum = 0;
    for (int i = 0; i < len; i++) {
        if (this.get(i) && extendedBitSet.get(i)) {
            sum++;
        }
    }

    return sum % 2;
}

Simplifying Complex Methods

A more complex situation arises in the toByteArray method. It is incredibly difficult to analyze. It took me about 20 minutes to understand its functioning. Now imagine a project composed of a thousand classes, each containing such methods. Managing such code would be nearly miraculous. Let’s look at the method’s code:

public byte[] toByteArray() {
    int bytesNumber = 0;

    if (fixedLength % 8 == 0) {
        bytesNumber = fixedLength / 8;
    } else {
        bytesNumber = fixedLength / 8 + 1;
    }

    byte[] arr = new byte[bytesNumber];

    for (int j = bytesNumber - 1, k = 0; j >= 0; j--, k++) {
        for (int i = j * 8; i < (j + 1) * 8; i++) {
            if (i == fixedLength) {
                break;
            }
            if (get(i)) {
                arr[k] += (byte) Math.pow(2, i % 8);
            }
        }
    }
    return arr;
}

The toByteArray method returns the byte representation of bit sequences — a 14-bit sequence can be represented using 2 bytes; a 23-bit sequence can be presented using 3 bytes.

The algorithm can be described in the following steps:

  • Determine the number of bytes.
  • For each byte’s worth of bits (or several bits in the case of incomplete bytes), perform the following operation:
    • Check the value of each bit.
    • If a bit has a value of 1, add the corresponding power of two (resulting from the bit’s position within the byte) to the byte’s value.

Why not express this algorithm programmatically? This is often forgotten. As programmers, we try to express our ideas concisely, leading to unreadable solutions that neither others nor we can understand quickly. The ideal is to aim for unraveling the creator’s intention at a glance without delving into algorithmic details. Look at another implementation of the toByteArray method:

public byte[] toByteArray() {
    int bytesCount = computeBytesCount();
    byte[] byteArray = new byte[bytesCount];

    for (int i = 0; i < bytesCount; ++i) {
        int byteNumber = bytesCount - i - 1;
        byteArray[byteNumber] = computeByteValue(i);
    }

    return byteArray;
}

The most important change involves directly expressing the algorithm at a general level, enabling someone to quickly understand the creator’s intention. The helper method computeBytesCount finds the number of bytes needed. The loop performs the operation of calculating the byte’s value and storing the result for each byte. Isn’t such a notation much simpler? Even if all algorithm elements are not visible, the more curious can always peek into the computeBytesCount and computeByteValue methods.

Helper Methods

These methods might look as follows:

private byte computeByteValue(int byteNumber) {
    int firstBitPosition = byteNumber * 8;
    int lastBitPosition = (byteNumber + 1) * 8 - 1;

    byte byteValue = 0;

    for (int i = this.nextSetBit(firstBitPosition);
         i >= firstBitPosition && i <= lastBitPosition;
         i = this.nextSetBit(i + 1)) {

        int currentBitPosition = i - firstBitPosition;

        if (get(i) == true) {
            byteValue += (byte) Math.pow(2, currentBitPosition % 8);
        }
    }

    return byteValue;
}

private int computeBytesCount() {
    int bytesCount = 0;

    if (fixedLength % 8 == 0) {
        bytesCount = fixedLength / 8;
    } else {
        bytesCount = fixedLength / 8 + 1;
    }

    return bytesCount;
}

The code hasn’t become significantly simpler, but it is much easier to analyze and understand. Refactoring theory calls these actions method extraction. We went further by slightly modifying the algorithm to make it more readable. Consider the lines:

int firstBitPosition = byteNumber * 8;
int lastBitPosition = (byteNumber + 1 ) * 8 - 1;

These are actually the extraction of very cryptic expressions from the loop:

for (int i = j * 8; i < (j + 1) * 8; i++) {

Isn’t the intention clear in that case? My experiences lead me to this conclusion:

Explicitly name complex code elements.

Instead of

j * 8

write

int firstBitPosition = byteNumber * 8;

This principle extends to conditional statements. Often, we encounter expressions that mask some logic. It’s worth directly expressing our intention. Readability increases immensely.

Instead of

if (index >= 0)

write

boolean isIndexInRange = (index >= 0);

if (isIndexInRange)

The code begins to read like a book! Programming is for people. Let’s make life easier for ourselves.

Conclusion

Here is the essential rule, embodying the essence of these considerations:

Write code in such a way that it reads like a novel. Use unambiguous yet simple names. Divide operations into logical parts and implement each in separate methods.

I think this is enough for one go. Enough to whet appetites and create a desire for more. Enough to show a glimpse of the garden of refactoring and its surroundings. I assure you that it’s an amazing place, providing immense joy and satisfaction.

Below, I provide the final version of the code that passes the included tests (of course, with the change considering non-static methods merge and boolMultiply).

Final Code

The final form of the example class (it’s worth comparing it with the initial form):

public class ExtendedBitSet extends BitSet {

    private int fixedLength = 0;

    public ExtendedBitSet(int size, String str) {
        super(size);

        fixedLength = size;
        initializeBitSet(str);
    }

    public ExtendedBitSet(String str) {
        this(str.length(), str);

        initializeBitSet(str);
    }

    private void initializeBitSet(String str) {
        int strLength = str.length();

        for (int i = 0; i < strLength; ++i) {
            if (str.charAt(strLength - 1 - i) == '1') {
                set(i);
            }
        }
    }

    public void merge(ExtendedBitSet extendedBitSet) {
        for (int i = extendedBitSet.nextSetBit(0); i >= 0;
             i = extendedBitSet.nextSetBit(i + 1)) {
            this.set(this.fixedLength + i);
        }
        this.fixedLength = this.fixedLength + extendedBitSet.fixedLength;
    }

    public int boolMultiply(ExtendedBitSet extendedBitSet) {
        int len = 0;

        if (this.fixedLength < extendedBitSet.fixedLength) {
            len = this.fixedLength;
        } else {
            len = extendedBitSet.fixedLength;
        }

        int sum = 0;

        for (int i = 0; i < len; i++) {
            if (this.get(i) && extendedBitSet.get(i)) {
                sum++;
            }
        }

        return sum % 2;
    }

    public byte[] toByteArray() {
        int bytesCount = computeBytesCount();
        byte[] byteArray = new byte[bytesCount];

        for (int i = 0; i < bytesCount; ++i) {
            int byteNumber = bytesCount - i - 1;
            byteArray[byteNumber] = computeByteValue(i);
        }

        return byteArray;
    }

    private byte computeByteValue(int byteNumber) {
        int firstBitPosition = byteNumber * 8;
        int lastBitPosition = (byteNumber + 1) * 8 - 1;

        byte byteValue = 0;

        for (int i = this.nextSetBit(firstBitPosition);
             i >= firstBitPosition && i <= lastBitPosition;
             i = this.nextSetBit(i + 1)) {

            int currentBitPosition = i - firstBitPosition;

            if (get(i) == true) {
                byteValue += (byte) Math.pow(2, currentBitPosition % 8);
            }
        }
        return byteValue;
    }

    private int computeBytesCount() {
        int bytesCount = 0;

        if (fixedLength % 8 == 0) {
            bytesCount = fixedLength / 8;
        } else {
            bytesCount = fixedLength / 8 + 1;
        }
        return bytesCount;
    }

    public String convertToBitString(int size) {
        char[] resultArray = new char[size];

        for (int i = 0; i < size; ++i) {
            resultArray[i] = '0';
        }

        for (int i = this.nextSetBit(0); i >= 0; i = this.nextSetBit(i + 1)) {
            resultArray[size - 1 - i] = '1';
        }
        return new String(resultArray);
    }

    public String convertToBitString() {
        return convertToBitString(this.fixedLength);
    }
}

Additional: Class Test

public class ExtendedBitSetTest extends TestCase {

    public void testConstructorSizeAndString() throws Exception {

        assertEquals("{0, 2}", new ExtendedBitSet(10, "101").toString());
        assertEquals("0000000101", new ExtendedBitSet(10, "101").convertToBitString());
        assertEquals("0000001011", new ExtendedBitSet(10, "1011").convertToBitString());
        assertEquals("0010001011", new ExtendedBitSet(10, "10001011").convertToBitString());
        assertEquals("{0, 1, 3, 7}", new ExtendedBitSet(10, "10001011").toString());
        assertEquals("{0}", new ExtendedBitSet(10, "001").toString());
        assertEquals("0000000001", new ExtendedBitSet(10, "001").convertToBitString());
        assertEquals("00000000001", new ExtendedBitSet(10, "001").convertToBitString(11));
    }

    public void testMerge() throws Exception {

        ExtendedBitSet extendedBitSet1 = new ExtendedBitSet(10, "1");
        ExtendedBitSet extendedBitSet2 = new ExtendedBitSet(10, "1");

        assertEquals("0000000001", extendedBitSet1.convertToBitString());
        assertEquals("0000000001", extendedBitSet2.convertToBitString());

        ExtendedBitSet mergedBitSet = ExtendedBitSet.merge(extendedBitSet1, extendedBitSet2);
        String mergedString = mergedBitSet.convertToBitString();

        assertEquals("00000000010000000001", mergedString);
        assertEquals("{0, 10}", mergedBitSet.toString());
        assertTrue(mergedBitSet.get(0) == true);
    }

    public void testBoolMultiple() throws Exception {

        ExtendedBitSet extendedBitSet1 = new ExtendedBitSet(3, "1");
        ExtendedBitSet extendedBitSet2 = new ExtendedBitSet(10, "1");

        assertEquals(1, ExtendedBitSet.boolMultiply(extendedBitSet1, extendedBitSet2));

        extendedBitSet1 = new ExtendedBitSet(1000, "1");
        extendedBitSet2 = new ExtendedBitSet(2, "1");

        assertEquals(1, ExtendedBitSet.boolMultiply(extendedBitSet1, extendedBitSet2));

        extendedBitSet1 = new ExtendedBitSet(10, "1");
        extendedBitSet2 = new ExtendedBitSet(10, "1");

        assertEquals(1, ExtendedBitSet.boolMultiply(extendedBitSet1, extendedBitSet2));

        extendedBitSet1 = new ExtendedBitSet(10, "1");
        extendedBitSet2 = new ExtendedBitSet(10, "10");

        assertEquals(0, ExtendedBitSet.boolMultiply(extendedBitSet1, extendedBitSet2));

        extendedBitSet1 = new ExtendedBitSet(10, "10");
        extendedBitSet2 = new ExtendedBitSet(10, "10");

        assertEquals(1, ExtendedBitSet.boolMultiply(extendedBitSet1, extendedBitSet2));

        extendedBitSet1 = new ExtendedBitSet(10, "110");
        extendedBitSet2 = new ExtendedBitSet(10, "110");

        assertEquals(0, ExtendedBitSet.boolMultiply(extendedBitSet1, extendedBitSet2));
    }

    public void testToByteArray() throws Exception {

        ExtendedBitSet extendedBitSet = new ExtendedBitSet("100000110");
        byte[] toByteArray = extendedBitSet.toByteArray();

        assertEquals(1, toByteArray[0]);
        assertEquals(6, toByteArray[1]);

        extendedBitSet = new ExtendedBitSet("10111111111");

        toByteArray = extendedBitSet.toByteArray();

        assertEquals(5, toByteArray[0]);
        assertEquals(-1, toByteArray[1]);
    }
}

(Text translated and moved from original old blog automatically by AI. May contain inaccuracies.)

Tags

Related Posts

You Shouldn't Run with a Broken Leg!

You Shouldn’t Run with a Broken Leg!

Clear? Obvious? It seems so, yet we continuously run with broken legs…

Read More

The Big Lie About Clean Code and Unit Testing

Introduction

Many people are convinced that waking up early, say at 5:00 AM, is an unrealistic fantasy for them. They believe that their nature and their body are constructed in such a way that it’s impossible. I used to belong to this group of people.

Read More

The Scrum You Don't Know

But we don’t have a Product Owner! The Product Owner is unavailable! We are working on several projects at the same time! Our deadlines are getting delayed. Testers do not have time to test our code.

Read More