current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

I have a class that contains defines 'magic' numbers at compile time such as basic array sizes, fixed ip address, etc. so that I have one place to edit if changes are required.

In this nested class I need to define tuples of maximum and minimum values that correspond to each of 4 bands of an IR camera. I'd like to use the data like this (for readability): 

int x = lutBounds[3].min;

Is my code below considered 'good practice'? If not, why, and what would be better. I'm completely self-taught in Java but try to use 'best practices' when I can recognize them.

/**
 * LUTBounds holds a band datum for the upper and lower bounds of interest.
 * @author programmer
 *
 */
 static class LUTBounds {
    final int   min;
    final int   max;

    LUTBounds(final int min, final int max) {
        this.min = min;
        this.max = max;

    }

}

// usage: int x = lutBounds[3].min;
final static LUTBounds[]    lutBounds   = { new LUTBounds(14_123, 30_123),
        new LUTBounds(14_922, 40_123), new LUTBounds(14_815, 41_123),
        new LUTBounds(14_300, 40_123) };
share|improve this question

2 Answers 2

up vote 4 down vote accepted

I would suggest an enum: they're made to encapsulate a set of distinct values known at compile time. For example, this might be the code you have as a result:

public enum CameraInformation {

    First(14_123, 30_123, "http://google.com"),
    Second(14_922, 40_123, "http://google.com"),
    Third(14_815, 41_123, "http://google.com"),
    Fourth(14_300, 40_123, "http://google.com");

    private final int min, max;
    private final String address;

    CameraInformation(int min, int max, String address) {
        this.min = min;
        this.max = max;
        this.address = address;
    }

    public int getMin() {
        return min;
    }

    public int getMax() {
        return max;
    }

    public String getAddress() {
        return address;
    }
}

The main reason for doing this is because I don't like the 0-based array index with the 1-based cameras: lutBounds[3] refers to camera 4 instead of 3, thus losing some expressiveness even for people familiar with this.

Using an enum also takes care of you having to choose a place to create those boundary values and passing them around: the enum is created once, stores its values in a central place and can be used anywhere you want.

Usage is easy:

int min = CameraInformation.Third.getMin();
share|improve this answer
    
I agree with the numbering. The engineer who created the camera counts from zero when sending the number but commands to the camera start with 1. It already bit me once! –  Nate Lockwood 2 days ago
up vote 3 down vote

In no case I'd recommend an array. Arrays are mutable and your final static LUTBounds[] is no constant. Guava's ImmutableList is the way to go (unless you prefer an enum). Because of the 1-based problem, I wouldn't expose the list, but rather a getter getLutBoundByOneBasedIndex(int) or something similarly terribly named, so I'd be always reminded of the insanity.

Given that the bounds probably never change, I'd prefer an enum just like in the other answer. Just don't forget that ordinal() is -based, in case you ever need it. Using project Lombok (which I'm a big fan of), you can shrink it considerably.

@RequiredArgsConstructor @Getter
public enum CameraInformation {
    First(14_123, 30_123, "http://google.com"),
    Second(14_922, 40_123, "http://google.com"),
    Third(14_815, 41_123, "http://google.com"),
    Fourth(14_300, 40_123, "http://google.com");

    private final int min, max;
    private final String address;
}
share|improve this answer
    
I forgot that I could iterate though the enums with Java's for-loop. I'll look at lombok. –  Nate Lockwood yesterday

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.