Handling an IRController

Question

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};
    //Nummernfeld und Navigation durch das Menü
    public static final byte[] NUMBER_ONE = {2,0,0,43,0,0,0,0,2,0,0,0,0,0,0,0};
    public static final byte[] UP = {2,0,0,82,0,0,0,0,2,0,0,0,0,0,0,0};
    public static final byte[] NUMBER_THREE = {2,-128,0,0,0,0,0,0,2,0,0,0,0,0,0,0};
    public static final byte[] LEFT = {2,0,0,80,0,0,0,0,2,0,0,0,0,0,0,0};
    public static final byte[] ENTER = {2,0,0,40,0,0,0,0,2,0,0,0,0,0,0,0};
    public static final byte[] RIGHT = {2,0,0,79,0,0,0,0,2,0,0,0,0,0,0,0};
    public static final byte[] NUMBER_SEVEN= {2,1,0,18,0,0,0,0,2,0,0,0,0,0,0,0};
    public static final byte[] DOWN = {2,0,0,81,0,0,0,0,2,0,0,0,0,0,0,0};
    public static final byte[] ESCAPE = {2,0,0,41,0,0,0,0,2,0,0,0,0,0,0,0};
    public static final byte[] ZERO = {2,0,0,0,0,0,0,0,2,4,0,43,0,0,0,0};
    //Lautstärkeregelung
    public static final byte[] VOL_UP = {3,-23,0};
    public static final byte[] VOL_DOWN = {3,-22,0};
    public static final byte[] MUTE = {3,-30,0};
}

This class represents the bytcode for every key

public class Key_Action {

    private static final Runnable SHUTDOWN = new Runnable(){
        @Override
        public void run() {
            try {
                ShutdownInfoController isc = new ShutdownInfoController();
                Window shtdnWin = new Window("/app/view/ShutdownInfoView.fxml","Info zum Herunterfahren",
                              isc,false);
            } catch (IOException ex) {
                Logger.getLogger(Keys.class.getName()).log(Level.SEVERE, null, ex);
            }
        }  
    };

    //Ruft das neue Fenster auf
    public static void callShutdown(){
        Platform.runLater(SHUTDOWN);
    }

    //PC wird heruntergefahren
    public static void actShutdown() throws IOException{
        String cmdtext = "shutdown.exe /s /hybrid /t 0 "; // Befehl um PC herunterzufahren
        Runtime.getRuntime().exec(cmdtext); // Befehl wird ausgeführt;
    }
}

This class performs some action when a button is pressed.

My question is: Is this code design ok? Or is it a bad style?

Answer 1

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

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

   Then you can iterate it over:

   for (Map.Entry<byte[], Runnable> entry: commands) {
       if (Array.equals(entry.getKey(), recData)) {
           Platform.runLater(entry.getValue());
       }
   }
   

2. 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.

3. 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)

4. 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

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