patternjavaMinor
Variable length integer encoding in Java
Viewed 0 times
lengthjavaencodingvariableinteger
Problem
I have coded up the variable length integer encoding aka VLQ in Java. It works with any base in
I've tested the code to a certain extent (unit tests accompany the code) though it is easy to miss a few cases, especially when manipulating bits. It'll be be helpful if more pairs of eyes look at the code too.
The code is hosted on github too.
updated to add argument validation in
```
package me.soubhik;
import org.apache.commons.lang3.StringUtils;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Iterator;
/**
* Created by soubhik on 06-08-2016.
*/
public class IntegerEncoding {
public static interface IntegerCode {
//index==0 => Most Significant
public byte[] encode(int x);
public int decode(byte[] code);
public byte[] add(byte[] a, byte[] b);
public byte[] sub(byte[] a, byte[] b);
public byte[] mult(byte[] a, byte[] b);
public boolean isValid(byte[] a);
public byte[] fromDecimalString(String decimalString);
default public String toBinaryString(byte[] code) {
StringBuilder builder = new StringBuilder();
for (byte c: code) {
String byteAsString = String.format("%8s", Integer.toBinaryString(c & 0xff)).replace(' ', '0');
builder.append(byteAsString);
builder.append(' ');
}
return builder.toString().trim();
}
default public byte[] fromBinaryString(String byteString) {
assert (StringUtils.isNotBlank(byteString));
String[] byteStringArray = byteString.split(" ");
byte[] code = new byte[byteStringArray.length];
int idx = 0;
(1, 128]. I've implemented the usual encode/decode methods and also the addition and multiplication for this encoding. Single byte multiplication uses shift-and-add and multi-byte ones uses Karatsuba algorithm.I've tested the code to a certain extent (unit tests accompany the code) though it is easy to miss a few cases, especially when manipulating bits. It'll be be helpful if more pairs of eyes look at the code too.
The code is hosted on github too.
updated to add argument validation in
encode(). also added test cases for large (> Long.MAX_VALUE) integers.```
package me.soubhik;
import org.apache.commons.lang3.StringUtils;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Iterator;
/**
* Created by soubhik on 06-08-2016.
*/
public class IntegerEncoding {
public static interface IntegerCode {
//index==0 => Most Significant
public byte[] encode(int x);
public int decode(byte[] code);
public byte[] add(byte[] a, byte[] b);
public byte[] sub(byte[] a, byte[] b);
public byte[] mult(byte[] a, byte[] b);
public boolean isValid(byte[] a);
public byte[] fromDecimalString(String decimalString);
default public String toBinaryString(byte[] code) {
StringBuilder builder = new StringBuilder();
for (byte c: code) {
String byteAsString = String.format("%8s", Integer.toBinaryString(c & 0xff)).replace(' ', '0');
builder.append(byteAsString);
builder.append(' ');
}
return builder.toString().trim();
}
default public byte[] fromBinaryString(String byteString) {
assert (StringUtils.isNotBlank(byteString));
String[] byteStringArray = byteString.split(" ");
byte[] code = new byte[byteStringArray.length];
int idx = 0;
Solution
Let me get two things out of the way: I won't even bother to comment on your naming, let alone comment on the fact that you're abusing
Assert is not argument validation
Using assert to validate arguments is bad practice. Instead of having an
Quoting the JLS on
In light of this, assertions should not be used for argument checking in public methods. Argument checking is typically part of the contract of a method, and this contract must be upheld whether assertions are enabled or disabled.
Minor and Major nitpicks
-
-
-
Binary operators should always have a space on either side. I see one violation in #decode (
-
Your methods are incredibly long. Additionally a significant number of variables is declared in them. Most of them could be smaller in scope if done correctly, in other places you should extract smaller methods to do less abstract work for you.
Testing
Your testing code is one huge nightmare.
All of this means that your test-suite is unusable. Utterly unusable for basically anything that's professional software development.
To fix this I strongly suggest you look into unit-testing frameworks like JUnit and TestNG
IntegerEncoding as a namespace.Assert is not argument validation
Using assert to validate arguments is bad practice. Instead of having an
AssertionError (which is dangerous) you should throw one of IllegalArgumentException, NullPointerException or in rare cases IllegalStateException.Quoting the JLS on
assert (§14.10):In light of this, assertions should not be used for argument checking in public methods. Argument checking is typically part of the contract of a method, and this contract must be upheld whether assertions are enabled or disabled.
Minor and Major nitpicks
-
ArrayList bytes = new ArrayList(); should instead be declared as List. Favor interfaces over implementations-
StringUtils#isNotBlank introduces a huge dependency. If your code only uses that it's significantly more effective to just rely on Objects#requireNonNull and String#trim().length() to check the same thing (in even better granularity).-
Binary operators should always have a space on either side. I see one violation in #decode (
result += b*multiplier). Use your IDE's linting tools!-
Your methods are incredibly long. Additionally a significant number of variables is declared in them. Most of them could be smaller in scope if done correctly, in other places you should extract smaller methods to do less abstract work for you.
Testing
Your testing code is one huge nightmare.
- You're not automating anything
- You're not using a Test-Framework to handle basic setup and teardown for you
- You have tests with over 50 LoC for a single functionality
- You have magic strings and magic numbers all over the place
- You have a 15 line comment in a test...
- And you're verifying significant portions of output by manually checking sysouts ...
All of this means that your test-suite is unusable. Utterly unusable for basically anything that's professional software development.
To fix this I strongly suggest you look into unit-testing frameworks like JUnit and TestNG
Context
StackExchange Code Review Q#139218, answer score: 4
Revisions (0)
No revisions yet.