Hello everybody,

I have a xml socket server which get data from xml sockets out of actionscript3. Now I'm having a lot of if statements. Can I use another way to make my code more clean? Ivibot is a piece of hardware my school provided. LedBlink is a runnable class that let's a led blinking on a specific port. Engine1 and Engine2 are also Runnable classes that let the engine run for like 30 sec. By the way I'm dutch so that's why you see dutch words. Here some translation Kleur is Color and kast is Closet. Thanks in advance for your help :)

Here below you see the server class:

package gc;

import HW.*;
import java.io.*;
import java.net.*;
import javax.swing.JTextArea;
import usb.ivi.DeviceNotFoundException;
import usb.ivi.*;

class SimpleServer {

    private static SimpleServer server;
    private static GUI gui;
    private static Ivi ivi = new Ivi();
    private int counter = 0;
    private int counter2 = 0;

    private static final byte EOF = (byte) 0x00;

    private ServerSocket serverSocket; //New ServerSocket
    private Socket childSocket; //New Socket
    private BufferedReader readerIn;
    private OutputStream socketOut;

    public static void main(String[] args) {

     int port = 8090;

            try {
                port = Integer.parseInt(args[0]);
            }
            catch(ArrayIndexOutOfBoundsException e) {
            // Catch exception and keep going.
            }

        gui = new GUI();
        gui.main(args);
        server = new SimpleServer(port);
        

    }

    private SimpleServer(int port) {

        System.out.println(" << Starting XML OwlServer on Port: " + port + " >>");
        gui.setMsgText(" << Starting XML OwlServer on Port: " + port + " >>");

        try {

            serverSocket = new ServerSocket(port);

            childSocket = serverSocket.accept();
            System.out.println("The connection is accepted.");
            gui.setMsgText("The connection is accepted.");
            InputStream in = childSocket.getInputStream();

            socketOut = childSocket.getOutputStream();
            out("<msg>Send <exit /> to do an exit.</msg>");

            while (true) {

                String line = "";
                int c;
                while( (c = in.read()) != 0 )
                    line += (char)c;
                
                
                if( line.length() < 1 )
                    continue;

                //readerIn.read(); // read extra EOF
                System.out.println("Received input string: \"" + line + "\"");

                gui.setMsgText("Received input string: \"" + line + "\"");
                //System.out.println("UTF-8 bytes: " + asHex(line));
                

                line = line.trim();

                if(line.equalsIgnoreCase("<exit/>")) {
                    out("Bye, your connection will be closed.");
                    gui.setMsgText("Bye, your connection will be closed.");
                    System.exit(0);
                    break;
                }

                if(line.equalsIgnoreCase("Kast 4")) {
                    gui.setMsgText("Boekenkast 4 doe iets!");
                    new Engine2(50);
                }

                if(line.equalsIgnoreCase("Kleur 3")){
                        System.out.println("rood");
                        new LedBlink(0);
                    }

                    if(line.equalsIgnoreCase("Kleur 4")){
                        new LedBlink(1);
                    }

                    if(line.equalsIgnoreCase("Kleur 5")){
                        new LedBlink(2);
                    }
                
                if(line.equalsIgnoreCase("Kast 6")) {
                        new Engine1(50);
                }

                if(line.equalsIgnoreCase("Kleur 0")){
                        System.out.println("rood");
                        new LedBlink(0);
                    }

                    if(line.equalsIgnoreCase("Kleur 1")){
                        new LedBlink(1);
                    }

                    if(line.equalsIgnoreCase("Kleur 2")){
                        new LedBlink(2);
                    }

                if(line.equalsIgnoreCase("<motorstop/>")){
                    System.out.println("Stop engine!");
                    gui.setMsgText("Stop engine!");

                    ivi.stopBeweging();
                }

                if(line.equalsIgnoreCase("<motor1stop/>")){
                    System.out.println("Stop engine1!");
                    gui.setMsgText("Stop engine1!");

                    ivi.stopMotor1();
                }

                if(line.equalsIgnoreCase("<motor2stop/>")){
                    System.out.println("Stop engine2!");
                    gui.setMsgText("Stop engine2!");

                    ivi.stopMotor2();
                }

               if(line.equalsIgnoreCase("<lichtuit/>")){
                   System.out.println("Put out the lights!");
                   gui.setMsgText("Put out the lights!");

                   ivi.dLeds_off();
                   ivi.bLeds_off();
               }
            }

            childSocket.close();
            socketOut.close();
            serverSocket.close();

        }
        catch(IOException e) {
            e.printStackTrace();
        }

    }

    private void out(String str) {

        System.out.println(">> " + str);
        System.out.println("UTF-8 bytes: " + asHex(str) + " 00");

        try {
            byte[] strBytes = str.getBytes("UTF-8");
            socketOut.write(strBytes);
            socketOut.write(EOF);
            socketOut.flush();
        }
        catch(UnsupportedEncodingException e) {
            // won't happen
        }
        catch(IOException e){
            throw new RuntimeException(e);
        }

    }

    protected String asHex(String str) {

        StringBuffer sb = new StringBuffer();

        try {

            byte[] bytes = str.getBytes("UTF-8");
            for(int i = 0; i < bytes.length - 1; i++) {
                sb.append(hexByte(bytes[i]));
                sb.append(" ");
            }

            sb.append(hexByte(bytes[bytes.length - 1]));

        }
        catch(UnsupportedEncodingException e) {
            // won't happen, UTF-8 is supported
        }

        return sb.toString();

    }

    protected String hexByte(byte b) {
        int i = b & 0xFF;
        String hex = Integer.toHexString(i).toUpperCase();
        if(i < 0x10) hex = "0" + hex;
        return hex;
    }

}

What you really need is a switch statement using a String expression. Unfortunately you'll have to wait for Java 7 to get one. Until then you can set up an enum with all the cases, convert the input String to enum with valueOf(...), then switch on the enum. IMHO this is a particularly good formal approach.
Alternatively
if you write methods with the same name as the input String values you can call the methods "by name" using the Reflection API. This leads to very compact and readable code, but sacrifices some compile-time checking.

Ok thanks for your post :), I forget to mention that some if statements needs to works simultaneously is that also possible with an enum. I have already worked with enum, but I only used one state at a time. Thanks in advance really appreciate it.

This is one of those times when I wish that functions were first-class citizens in Java. I'd really like to just stick those method calls into a hash table right about now. Unfortunately, you'd have to set up a class to encapsulate that funtionality, and managing all of those Command objects would probably cause more nuisance than it would save.

I was trying to find a way to define an object so you could manage the functionalities in a properties file, to keep that stuff out of the code - that would streamline things a little - but it just looks like it'd be a mess no matter what.

Hi Jon
It only takes one line of code to call a method given the name of the method in a String (and only a little more to get that info from an @annotation). Putting Methods into a HashTable and calling them is similarly easy. I'm personally very excited about the way you can extend Java, and eliminate boiler-plate code, using Reflection. We can take this further, here or elsewhere, if you're interested.

hey james thanks for your response can you give me an code example, because I don't understand it totally. Thanks in advance. Also i'm very happy that people are trying to help me :)

Thanks for the tip, James. I spent an hour with the Reflection API, which is something I'd been meaning to do, and I was able to come up with something that puts methods into a hashmap and retrieves them, so the first part of your promise is delivered. It's not exactly neat or tidy, though, so if you want to have a blather about that, I'd certainly like to know more.

Here's the sort of "hello, world" version, I think this is the sort of thing you're talking about doing.

("MeandJava", I hope you'll find this a useful example, to get an idea of what James is talking about. I'd point you to the Sun tutorials, and the APIs for the java.lang.Class and java.lang.reflect.Method, for the real story)

import java.util.HashMap;
import java.lang.reflect.Method;
import java.lang.Class;
import java.lang.reflect.InvocationTargetException;
import java.util.Scanner
import java.util.Scanner;
import java.util.HashMap;
import java.lang.reflect.Method;
import java.lang.Class;
import java.lang.reflect.InvocationTargetException;

public class Reflector
{
   public static void main(String[] args)
   {
      Reflector r = new Reflector();  // need to instantiate an object to call the methods on

      Class ref = Reflector.class;  // also need a Class object

      HashMap <String, Method> methods= new HashMap<String, Method>();

      Scanner scan = new Scanner(System.in);
      
      try{
      methods.put("a",  ref.getMethod("printA"));  // get the method of no arguments of this name, and 
      methods.put("b",  ref.getMethod("printB"));  // put it in the hashmap
      methods.put("c",  ref.getMethod("printC"));
      }
   
      catch(NoSuchMethodException e)  // checked exception
      {
         e.printStackTrace();
      }
      
      
      System.out.print("Enter 'a', 'b', or 'c': ");
      String input= scan.nextLine();


      try{
         methods.get(input).invoke(r);  //get the method out of the map and invoke it
      }
      catch(IllegalAccessException e)  // checked exception
      {
         e.printStackTrace();
      }
      catch(InvocationTargetException e) // checked exception
      {
         e.printStackTrace();
      }
   }


   public void printA()
   {
      System.out.println("A");
   }
   public void printB()
   {
      System.out.println("B");
   }
   public void printC()
   {
      System.out.println("C");
   }
}

Edited 5 Years Ago by jon.kiparsky: n/a

That's exactly what I meant, and it's pretty neat & tidy even in that demo form.
As a mapping of commands to methods it may sometimes be possible to set it up so the command String is the same as the method name (or can be transformed into it by (eg) replacing any blanks with _), in which case you don't need the Map and what's left is about as simple as anything can be.
I have further things I can share about passing params, returning values, and increasing the security in the command -> method transforming case, if these become relevant.
J

Thanks you guys :) This is exactly what I need thanks for the support!

How can I change jon his example so it works with different runnable classes. I tried to make some more objects and class objects but I will get the noSuchMethod Exception. here is a snippet of my code:

private SimpleServer(int port) {

        System.out.println(" << Starting server on Port: " + port + " >>");

        LedBlink l = new LedBlink(0);  // need to instantiate an object to call the methods on
        Engine1 e1 = new Engine1(30);
        Engine2 e2 = new Engine2(30);


      Class led = LedBlink.class;  // also need a Class object
      Class eng1 = Engine1.class;
      Class eng2 = Engine2.class;

      HashMap <String, Method> methods= new HashMap<String, Method>();

      try{

      methods.put("Color 0", led.getMethod("LedBlink"));
      methods.put("Color 1", led.getMethod("LedBlink"));
      methods.put("Color 2", led.getMethod("LedBlink"));
      methods.put("Color 3", led.getMethod("LedBlink"));
      methods.put("Color 4", led.getMethod("LedBlink"));
      methods.put("Color 5", led.getMethod("LedBlink"));

      methods.put("button 4", eng1.getMethod("Engine1"));
      methods.put("button 6", eng2.getMethod("Engine2"));

      }

      catch(NoSuchMethodException e)  // checked exception
      {
         e.printStackTrace();
      }

        try {

            serverSocket = new ServerSocket(port);

            childSocket = serverSocket.accept();
            System.out.println("The connection is accepted.");
            InputStream in = childSocket.getInputStream();

            socketOut = childSocket.getOutputStream();
            out("<msg>Send <exit /> to do an exit.</msg>");

           while (true) {

                String line = "";
                int c;
                while( (c = in.read()) != 0 )
                    line += (char)c;


                if( line.length() < 1 )
                    continue;

                //readerIn.read(); // read extra EOF
                System.out.println("Received input string: \"" + line + "\"");

                line = line.trim();

                if(line.equalsIgnoreCase("<exit/>")) {
                    out("Bye, your connection will be closed.");
                    gui.setMsgText("Bye, your connection will be closed.");
                    System.exit(0);
                    break;
                }

                try{
                    methods.get(line).invoke(l);
                    ......
                    ......
                    ......        

                }
                   catch(IllegalAccessException e)  // checked exception
                {
                e.printStackTrace();
                }
                catch(InvocationTargetException e) // checked exception
                {
                e.printStackTrace();
                }
            }

            childSocket.close();
            socketOut.close();
            serverSocket.close();

        }
        catch(IOException e) {
            e.printStackTrace();
        }

    }

So the method in the LedBlink class that you want to invoke is also called LedBlink?
If so, that's legal, but very confusing.
The parameter to the getMethod method is the name of a method, not the name of the class; maybe that's your problem.
At this stage we're assuming these methods do not take any parameters. Parameters aren't hard, but maybe one step at a time...

Hey James to be exact it is the constructor. I will paste the LedBlink class so you will understand what I'm trying to do. the LedBlink class implements Runnable as shown below. The usb device is a piece of hardware i got from my education. PortD is a row of IO Ports where my leds are to attached. Thanks in advance :)

package HW

import usb.ivi.*;

public class LedBlink implements Runnable {

    USB_ivi_device dev = new USB_ivi_device();
    Ivibot bot = new Ivibot(dev);
    Thread t;
    int port; // made variable

    /**
     *
     * @param port = io number of port D.
     * This constructor creates a new Thread and also starts it.
     * If something goes wrong it catches an exception.
     */

    public LedBlink(int port) {

        this.port = port;

        t = new Thread(this, "Blink Thread");
        t.start(); // Start the thread

        try {
        dev = new USB_ivi_device();

        } catch (Exception e){
            System.err.println("USB_IVI DLL Exception catched - device disabled");
            dev = null;
            return;
        }
    }

    /**
     * The method run tells the thread what to do. In this method the leds
     * get the command to blink. The led turns on, 500 millisec. waiting, leds
     * goes off, 500 millisec waiting and it starts again. This process repeats
     * 30 times.
     * If something goes wrong it catches an exception.
     */

    @Override
    public void run() {

        for (int i = 0; i < 15; i++) {
            dev.getPortD().setPinValue(port, true); 

            try{
            Thread.sleep(1000);
            } catch (Exception ex) {
                ex.printStackTrace();
            }
            dev.getPortD().setPinValue(port, false);

            try{
                Thread.sleep(1000);
            } catch (Exception ex) {
                ex.printStackTrace();
            }

        }
    }
}

OK. You can't call a constructor as if it was a method. The Reflection equivalent to the "new" keyword is the newInstance method in Class, but I wouldn't recommend going that way...

I would avoid using a constructor to control behaviour like that - it's a dead-end that's hard to move on from. Think about the constructor just returning an instance of LED (so with 6 LEDs you would have 6 instances) then having a public blink(boolean b) that tells the LED to start or stop blinking. You would create the timer in the constructor, and just start or stop it in the method.
That way you have a natural extension to, eg, turning each LED on or off (not blinking), or changing its brightness or colour, if that's possible.
In other words, make the LED class represent instances of LEDs that have various behaviours that you can start or stop by calling methods on that LED instance. This is quite different from a "LedBlink" class that represents a single behaviour.

This article has been dead for over six months. Start a new discussion instead.