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

Verifying validity of Intel.hex file records

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

Problem

I have a class called FirmwareFile which represents and Intel HEX format file. The file is constructed with the name, build time and build date, at a later point file data is retrieved from the database (another class handles this) and is passed in with the setData(byte[] data) method.

After the data is set, the method isValidData() should be called. This iterates through the data and analyses the hex records using their checksum byte. How is my code looking here? What can I do to improve it?

I suspect lines like the second one here aren't necessary, because I'm already dealing with a byte:

byte checksum = (byte) (sumLSB + 1);
checksum = (byte) (checksum & 0xFF);


Here's the whole class, which works perfectly well, but how can I improve it? For example, should I validate the data as it's passed in instead of with a separate method call?

```
public class FirmwareFile {

private String fileName;
private String buildTime;
private String buildDate;
private byte[] data = null;
private String[] stringData;
/*
* This file has been checked and verified as correct using the checksum
* included with each record. The checksum is computed by summing the
* decoded byte values, extracting the LSB of the sum, and calculating the
* two's complement of the LSB.
*/
private boolean validated = false;

/*
* The dataCorrupted variable is used to point towards corrupted records in
* the FirmwareFile data row. This should correspond to the same record line
* number in the original firmware.hex file. if the number is 0 and verified
* == true then the file can be trusted. if dataCorrupted > 0, then there
* are errors in the file located at the corresponding row number.
*/
private int dataCorrupted = 0;

final Charset charSet = Charset.forName(CommunicationState.CHARSET);
private Logger log = Logger.getLogger(FirmwareFile.class.getName());

public FirmwareFile(String f

Solution

When you call setData(), you fail to reset the validated flag to false and the dataCorrupted pointer. On that topic, what is the point of dataCorrupted? It is private, but write-only.

I am concerned about new String(data). Perhaps you meant to use charset.

These few lines are problematic:

System.out.println("Validating data, start: " + new Date());
    if(!validated && stringData != null && dataCorrupted == 0){

        for(int i = 1; i < stringData.length; i++){


Why print to System.out? You have a logger. Stuff like this is probably more appropriate for System.err anyway.

The condition should be inverted, so that you have an early return, to prevent the awkward else way down there.

The for loop is weird, as it starts with 1. If that is intentional, it needs a comment.

Code Snippets

System.out.println("Validating data, start: " + new Date());
    if(!validated && stringData != null && dataCorrupted == 0){

        for(int i = 1; i < stringData.length; i++){

Context

StackExchange Code Review Q#87848, answer score: 2

Revisions (0)

No revisions yet.