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

Handling an IRController

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

Problem

At the time I try to write some code to handle an IRController, I got this following classes:

public class UsbDataInputListener implements UsbPipeListener {

    @Override
    public void errorEventOccurred(UsbPipeErrorEvent upee) {
        throw new UnsupportedOperationException("Not supported yet."); //To change body of generated methods, choose Tools | Templates.
    }

    @Override
    public void dataEventOccurred(UsbPipeDataEvent upde) {
        final byte[] recData = upde.getData(); //Übertragene Bytes
        //Input wird überprüft
        //Aktion für die jeweilige Taste wird durchgeführt
        if(Arrays.equals(recData, Keys.POWER)){
            Key_Action.callShutdown();
        }
        ....
        ....
}


This class waits for a DataEvent (pressing a button on the controller):

```
public final class Keys {

public static final byte[] POWER = {4,1};
//Direkt Zugriff aus Musi,Spiele,Gallerie und Videos
public static final byte[] MUSIC = {2,5,0,4,0,0,0,0,2,5,0,30,0,0,0,0};
public static final byte[] GAME = {2,5,0,5,0,0,0,0,2,5,0,31,0,0,0,0};
public static final byte[] GALLERY = {2,5,0,6,0,0,0,0,2,5,0,32,0,0,0,0};
public static final byte[] VIDEO = {2,5,0,7,0,0,0,0,2,5,0,33,0,0,0,0};
//Steurung für Film und Musik
public static final byte[] BEFORE = {3,-74,0};
public static final byte[] BEFORE2 = {2,0,0,0,0,0,0,0,3,-74,0};
public static final byte[] NEXT = {3,-75,0};
public static final byte[] NEXT2 = {2,0,0,0,0,0,0,0,03,-75,0};
public static final byte[] BACKWARD = {3,-53,0};
public static final byte[] BACKWARD2 = {2,0,0,0,0,0,0,0,03,-53,0};
public static final byte[] FORWARD = {3,-54,0};
public static final byte[] FORWARD2 = {2,0,0,0,0,0,0,0,03,-54,0};
public static final byte[] PLAY = {3,-51,};
public static final byte[] PLAY2 = {2,0,0,0,0,0,0,0,03,-51,0};
public static final byte[] STOP = {3,-73,0};
public static final byte[] STOP2 = {2,0,0,0,0,0,0,0,03,-73,0};
/

Solution

-
I would try to create a data structure which stores both event data and required action. Something like this:

Map commands = new HashMap<>();
commands.put(Keys.POWER, SHUTDOWN);
...


Then you can iterate it over:

for (Map.Entry entry: commands) {
    if (Array.equals(entry.getKey(), recData)) {
        Platform.runLater(entry.getValue());
    }
}


-
It seems that the logger uses a wrong classname here:

Logger.getLogger(Keys.class.getName()).log(Level.SEVERE, null, ex);


It might mislead maintainers/bug hunters.

-
The comments is just a noise here, you should remove it:

throw new UnsupportedOperationException("Not supported yet."); //To change body of generated methods, choose Tools | Templates.


(Clean Code by Robert C. Martin: Chapter 4: Comments, Noise Comments)

-
Usually I try to avoid abbreviations like upde, shtdnWin, actShutdown. They are not too readable and I suppose you have autocomplete (if not, use an IDE, it helps a lot), so using longer names does not mean more typing but it would help readers and maintainers a lot since they don't have to remember the purpose of each variable - the name would express the programmers intent and would not force readers to decode the abbreviations every time they read/maintain the code.

See also: Clean Code by Robert C. Martin, Use Intention-Revealing Names, p18; Avoid Mental Mapping, p25


If you can’t pronounce it, you can’t discuss it without sounding like an idiot. “Well,
over here on the bee cee arr three cee enn tee we have a pee ess zee kyew int, see?” This
matters because programming is a social activity.

Source: Clean Code by Robert C. Martin, Use Pronounceable Names, p21

-
Key_Action does not follow the usual Java naming conventions. Classnames should be CamelCase, without underscore.

Code Snippets

Map<byte[], Runnable> commands = new HashMap<>();
commands.put(Keys.POWER, SHUTDOWN);
...
for (Map.Entry<byte[], Runnable> entry: commands) {
    if (Array.equals(entry.getKey(), recData)) {
        Platform.runLater(entry.getValue());
    }
}
Logger.getLogger(Keys.class.getName()).log(Level.SEVERE, null, ex);
throw new UnsupportedOperationException("Not supported yet."); //To change body of generated methods, choose Tools | Templates.

Context

StackExchange Code Review Q#49560, answer score: 2

Revisions (0)

No revisions yet.