patternjavaMinor
Creating a barcode bytewise
Viewed 0 times
creatingbarcodebytewise
Problem
I am currently working on a piece of legacy code that I inherited. The current Java version is 6, but this will change in the next few weeks to Java 7. I think I might have been working on it for too long as I get the impression that it can be made much more readable as I have many byte-wise operations, but cannot find a way to do so.
Do any of you have suggestions on how to improve readability on the following method?
(The class has some more methods of very different setup, however, not much more readable. Since this method can be understood and refactored on its own, I separated it from the class.)
```
/**
* Creates the barcode bytewise and writes it on the page referenced in the
* barcodeData object.
*
* @param barcodeData {@link objects.BarcodeData}
* @throws ConverterRuntimeException
*/
public void addBarcode2of5(BarcodeData barcodeData) throws ConverterRuntimeException {
try {
IAfpCmd cmdBpg = barcodeData.getPage().findFirstAfpCommand(SfRecordType.BPG);
CmdDef cmdBbc = new CmdDef(SfRecordType.BBC);
CmdDef cmdBog = new CmdDef(SfRecordType.BOG);
ByteData data;
cmdBpg.addAfpCmd(cmdBbc, true); // Begin Bar Code Object
cmdBbc.addAfpCmd(cmdBog, true); // Begin Object Environment Group
data = new ByteData(new byte[0], 0);
data.append(ByteConvUtil.int2u2Bytes(0x0343)); // Triplet 43
data.append(ByteConvUtil.int2u1Bytes(1));
data.append(ByteConvUtil.int2u2Bytes(0x084B)); // Triplet 4B
data.append(ByteConvUtil.int2u2Bytes(0));
data.append(ByteConvUtil.int2u2Bytes(barcodeData.getPageResolution().getXResolution().getResolutionInPelsPerInch() * 10)); // Auflösung in X-Richtung
data.append(ByteConvUtil.int2u2Bytes(barcodeData.getPageResolution().getYResolution().getResolutionInPelsPerInch() * 10)); // Auflösung in Y-Richtung
data.
Do any of you have suggestions on how to improve readability on the following method?
(The class has some more methods of very different setup, however, not much more readable. Since this method can be understood and refactored on its own, I separated it from the class.)
```
/**
* Creates the barcode bytewise and writes it on the page referenced in the
* barcodeData object.
*
* @param barcodeData {@link objects.BarcodeData}
* @throws ConverterRuntimeException
*/
public void addBarcode2of5(BarcodeData barcodeData) throws ConverterRuntimeException {
try {
IAfpCmd cmdBpg = barcodeData.getPage().findFirstAfpCommand(SfRecordType.BPG);
CmdDef cmdBbc = new CmdDef(SfRecordType.BBC);
CmdDef cmdBog = new CmdDef(SfRecordType.BOG);
ByteData data;
cmdBpg.addAfpCmd(cmdBbc, true); // Begin Bar Code Object
cmdBbc.addAfpCmd(cmdBog, true); // Begin Object Environment Group
data = new ByteData(new byte[0], 0);
data.append(ByteConvUtil.int2u2Bytes(0x0343)); // Triplet 43
data.append(ByteConvUtil.int2u1Bytes(1));
data.append(ByteConvUtil.int2u2Bytes(0x084B)); // Triplet 4B
data.append(ByteConvUtil.int2u2Bytes(0));
data.append(ByteConvUtil.int2u2Bytes(barcodeData.getPageResolution().getXResolution().getResolutionInPelsPerInch() * 10)); // Auflösung in X-Richtung
data.append(ByteConvUtil.int2u2Bytes(barcodeData.getPageResolution().getYResolution().getResolutionInPelsPerInch() * 10)); // Auflösung in Y-Richtung
data.
Solution
When interfacing with 'library' code, especailly when the library is a Java representation of some low-level interface, it is always somewhat ugly. What you see as having readability problems is true, but it's not your fault (mostly).
In circumstances like this, you have to transcode the data in one format, to another, and that other format is highly structured, and specialized. The only way for the code to make sense is if you have the specification for that other API in front of you, or know it really well. In cases like that, I always find it best for the code to represent the specification as much as possible. In this case, I think it does....
... this is even true for the 'magic numbers'. Normally magic numbers are 'bad', and your code is full of them.... but, if the specification contains things like: ... Repeating group consisting of xxa - 3 bytes, xxb - 3 bytes, xxc - 2 bytes, xxd 2 bytes, ..... then you really are doing things 'OK'.
In general, in these cases, the best you can accomplish is to contain the ugliness in a single wrapper library that does the transformation for you, and does not leak the ugliness out.
There are three things I think you can improve:
-
Do not reuse any variables. Specifically,
This way you can move things around a bit more, and you are guaranteed that a later refactor of the code won't mess things up.
-
You have special cases for
to just:
which is consistent with the rest of the code.
-
The special
In circumstances like this, you have to transcode the data in one format, to another, and that other format is highly structured, and specialized. The only way for the code to make sense is if you have the specification for that other API in front of you, or know it really well. In cases like that, I always find it best for the code to represent the specification as much as possible. In this case, I think it does....
... this is even true for the 'magic numbers'. Normally magic numbers are 'bad', and your code is full of them.... but, if the specification contains things like: ... Repeating group consisting of xxa - 3 bytes, xxb - 3 bytes, xxc - 2 bytes, xxd 2 bytes, ..... then you really are doing things 'OK'.
In general, in these cases, the best you can accomplish is to contain the ugliness in a single wrapper library that does the transformation for you, and does not leak the ugliness out.
There are three things I think you can improve:
-
Do not reuse any variables. Specifically,
data should be:ByteData dataOBD = new ByteData(new byte[0], 0);
dataOBD.append(....);
.....
cmdBbc.addAfpCmd(new CmdDef(SfRecordType.OBD,
dataOBD.getCompleteDataArray()), true);
ByteData dataBPP = new ByteData(new byte[0], 0);
dataBPP.append(....);
.....This way you can move things around a bit more, and you are guaranteed that a later refactor of the code won't mess things up.
-
You have special cases for
BBC and BOG. I would change the code:CmdDef cmdBbc = new CmdDef(SfRecordType.BBC);
CmdDef cmdBog = new CmdDef(SfRecordType.BOG);
cmdBpg.addAfpCmd(cmdBbc, true); // Begin Bar Code Object
cmdBbc.addAfpCmd(cmdBog, true); // Begin Object Environment Groupto just:
cmdBpg.addAfpCmd(new CmdDef(SfRecordType.BBC),true); // Begin Bar Code Object
cmdBbc.addAfpCmd(new CmdDef(SfRecordType.BOG), true); // Begin Object Environment Groupwhich is consistent with the rest of the code.
-
The special
\n\r newline in the exceptions.... are you sure it is necessary? And, if it is, why are you not using the system property or a constant for it. Also, you are not adding the actual stack to the exception (just the message). Your exception class ConverterRuntimeException should be able to take a Throwable constructor argument, and it should look like (note the ex at the end):throw new ConverterRuntimeException("Ein Fehler ist beim Erstellen des Barcode aufgetreten: " + NEWLINE + ex.getMessage(), ex);Code Snippets
ByteData dataOBD = new ByteData(new byte[0], 0);
dataOBD.append(....);
.....
cmdBbc.addAfpCmd(new CmdDef(SfRecordType.OBD,
dataOBD.getCompleteDataArray()), true);
ByteData dataBPP = new ByteData(new byte[0], 0);
dataBPP.append(....);
.....CmdDef cmdBbc = new CmdDef(SfRecordType.BBC);
CmdDef cmdBog = new CmdDef(SfRecordType.BOG);
cmdBpg.addAfpCmd(cmdBbc, true); // Begin Bar Code Object
cmdBbc.addAfpCmd(cmdBog, true); // Begin Object Environment GroupcmdBpg.addAfpCmd(new CmdDef(SfRecordType.BBC),true); // Begin Bar Code Object
cmdBbc.addAfpCmd(new CmdDef(SfRecordType.BOG), true); // Begin Object Environment Groupthrow new ConverterRuntimeException("Ein Fehler ist beim Erstellen des Barcode aufgetreten: " + NEWLINE + ex.getMessage(), ex);Context
StackExchange Code Review Q#51382, answer score: 6
Revisions (0)
No revisions yet.