5

I am writing a communication software that will talk to lab processes in the control department in my uni. The processes communicate over serial port and there will be a fair bit of bit checking/manipulation. I have written a helper class like the following:

public class Channel {

    public enum Kind {DIGITAL_IN, DIGITAL_OUT, ANALOG_IN, ANALOG_OUT, COUNTER_IN};

    private int resolution;
    private int number;
    private Kind kind;
    public byte[] bits;

    public Channel(Kind kind, int resolution, int number) throws Exception {
        if (resolution % 8 != 0) {
            throw new Exception("Resolution must be divisible by 8");
        }
        this.kind = kind;
        this.resolution = resolution;
        this.number = number;
        this.bits = new byte[resolution/8];
    }


    public int getResolution() {
        return resolution;
    }

    public int getNumber() {
        return number;
    }

    public Kind getKind() {
        return kind;
    }
}

My question now is if it would be considered bad practice in this case to have my byte[] declared as public? In my LabProcessProtocol class I will access these channels bits and change them according to what I get from the process on the serial port.

I have a hunch that Java is all about being private and using getters and setters but I'm not sure. It seems so convoluted in this case.

Thanks in advance.

evading
  • 3,032
  • 6
  • 37
  • 57

6 Answers6

9

Well, there's no absolute prohibition against public fields. If you feel that is the best solution, then don't be ashamed, go ahead an do it.

That said, stop and think about what you want to accomplish. Making everything private is not a goal in itself - the idea is to establish invariants, to make the code easier to understand and to work with.

So consider what you want to do with the byte[] - what operations will others want to perform on it? Consider having methods for these operations, that will be easier to understand and cleaner than exposing the field as public. Also, consider what operations you do not want to allow (that would be the part that establishes invariants). For example, direct field access would allow replacing the byte[] with another one of different length - you probably want to prevent that.

Or maybe so much happens to this byte[] that it deserves a (wrapping) class of its own? This all depends on how it is used.

In closing, there's no problem with starting with a simple public field. You can always refactor it later, once you have found a more appropriate solution.

Note: "You can always refactor it later" does not apply for classes which are part of a public API (i.e. you are writing a library to be used by other, outside projects). Design of public APIs (often just called "API design") is much harder than design of "internal" code, this is just one example. This probably does not apply in this case, I just wanted to point it out.

sleske
  • 81,358
  • 34
  • 189
  • 227
  • Thanks! That was helpful. It will be private. – evading Nov 03 '12 at 20:55
  • 1
    Thoughtful answer. Just one remark from me. Even though "You can always refactor it later" is probably true for OP it is not in other cases. If you are delivering code that is used by others (i.e. a framework or 'API') then once the field has been made public it will be used as such and cannot be hidden without breaking backwards compatibility. – Adriaan Koster Nov 03 '12 at 23:19
3

Generally it would be recommended to leave the byte[] field as private and implement methods to manipulate the field from outside the class. It's not that it's terrible practice to declare the field as public (at least imho), but leaving it private could avoid issues later on as you continue to develop/update the project. I think this describes what I mean pretty well.

Community
  • 1
  • 1
arshajii
  • 127,459
  • 24
  • 238
  • 287
3

Make it private, but don't just use getters and setters -- how should users be accessing your bits array? Are they always getting the first element, or are they always cycling through it? (ByteBuffer might give you some useful ideas here, or maybe you should just use ByteBuffer directly.) Provide methods to access it in the ways you intend users to use it; don't just provide getters and setters.

Louis Wasserman
  • 191,574
  • 25
  • 345
  • 413
2

I would expose the bits[] via accessors(getters/setters). By exposing the array via accessors you are at least funneling access to the array to a specific method. If you needed to make a change in the future you would not have a bunch of code relying upon direct access to this field, which provides you with slightly more flexibility.

Kevin Bowersox
  • 93,289
  • 19
  • 159
  • 189
  • 3
    I don't think just having getter&setter is significantly better than a public field - it boils down to the same thing. The only exception is a public API, there it might be worth it. Otherwise, if you want to give complete access to a field, just make it public. If you later need to change it, just refactor the using classes along with this class. – sleske Nov 03 '12 at 20:51
0

I you care about separating the implementation from the API. If you would like, in the future, when you will change something to the internals of the Channel class, other code that is using Channel API to still work without modifications, then you will use getters and setters and use private fields. In other cases you can use public fields. But, note that in these case even the name and the type of the bits field are implementations details. In my opinion the name and the type of your internal fields should not affect other classes.

dan
  • 13,132
  • 3
  • 38
  • 49
0

By making the fields private and using accessor and modifier methods you are following the encapsulation rule which prevents the fields from accidental change and thus development of a more fault tolerant application.

Leo The Four
  • 699
  • 9
  • 14