How to make this to OOP

Question

@Override
public void onMotionSensingEvent( MotionSensingEvent arg0) {

    double gradient = 1.38;
    double speed;

    float testpitch = 0;
    testpitch = arg0.getOrientation().getPitch();

    float testroll = 0;
    testroll = arg0.getOrientation().getRoll();


    if (testroll > 16 && flyingControl)
    {
        speed = gradient*testroll-22.1;
        System.out.println("kanan = " + speed);
        ardrone.goRight(speed);
    }
    else if (testroll < (-24) && flyingControl)
    {
        speed = gradient*testroll*-1 - 22.1;
        System.out.println("kiri = " + speed);
        ardrone.goLeft(speed);
    }
    else if (testpitch < (-20) && flyingControl)
    {
        System.out.println("go up");
        ardrone.up();
    }
    else if (testpitch > 20 && flyingControl)
    {
        System.out.println("go down");
        ardrone.down();
    }       
}

@Override
public void onExpansionEvent(ExpansionEvent e)
{
    JoystickEvent joy = getJoystickEvent(e);

    float gradient =64;
    float speed;

    float angle = 0;
    angle = joy.getAngle();
    float magnitude = joy.getMagnitude();


    if ((angle > 340 || angle < 20) && magnitude > 0.49 && flyingControl)
    {
        speed = gradient*magnitude-19;

        ardrone.forward(speed);
    }
    else if ((angle > 160 && angle < 200) && magnitude > 0.49 && flyingControl)
    {
        speed = gradient*magnitude-19;

        ardrone.backward(speed);
    }
    else if ((angle > 70 && angle < 110) && magnitude > 0.7 && flyingControl)
    {
        System.out.println("turn right ");
        ardrone.spinRight();
    }
    else if ((angle > 250 && angle < 290) && magnitude > 0.7 && flyingControl)
    {
        System.out.println("turnleft ");
        ardrone.spinLeft();
    }
}

I have this code, and I need help to remove all these IF-ELSE things out of the main class, make it into OOP but I don't know a good way to start with. My supervisor said something about listener but I didn't understand. fyi, I have those listener but I don't include them in this question.

so then, can anybody tell me the step to remove all these IF ELSE out of the main class? thanks a lot!!

Answer 1

how about put these logic in the getter or setter in your object? Say your have an object called plane, that speed is its private property, I suppose you can implement these logic in the getter or setter for the speed property.

public plane{

private double gradient = 1.38;
private double speed;
private float testroll = 0;

public plane(MotionSensingEvent arg0){
    testroll = arg0.getOrientation().getRoll();
}

public getSpeed(){
    this.speed =  gradient*testroll-22.1;
    return speed;
}

}

Answer 2

I make it bit cleaner. But there are more.

  @Override
    public void onMotionSensingEvent( MotionSensingEvent arg0) {

        double gradient = 1.38;
        double speed;

        float testpitch = 0;
        testpitch = arg0.getOrientation().getPitch();

        float testroll = 0;
        testroll = arg0.getOrientation().getRoll();

       if( flyingControl){

           if (testroll > 16 )
           {
               speed = gradient*testroll-22.1;
               System.out.println("kanan = " + speed);
               ardrone.goRight(speed);
           }
           else if (testroll < (-24) )
           {
               speed = gradient*testroll*-1 - 22.1;
               System.out.println("kiri = " + speed);
               ardrone.goLeft(speed);
           }
           else if (testpitch < (-20) )
           {
               System.out.println("go up");
               ardrone.up();
           }
           else if (testpitch > 20 )
           {
               System.out.println("go down");
               ardrone.down();
           }       

       }

    }

    @Override
    public void onExpansionEvent(ExpansionEvent e)
    {
        JoystickEvent joy = getJoystickEvent(e);

        float gradient =64;
        float speed;

        float angle = 0;
        angle = joy.getAngle();
        float magnitude = joy.getMagnitude();

        if( flyingControl){

        }
        if ((angle > 340 || angle < 20) && magnitude > 0.49)
        {
            speed = gradient*magnitude-19;

            ardrone.forward(speed);
        }
        else if ((angle > 160 && angle < 200) && magnitude > 0.49)
        {
            speed = gradient*magnitude-19;

            ardrone.backward(speed);
        }
        else if ((angle > 70 && angle < 110) && magnitude > 0.7 )
        {
            System.out.println("turn right ");
            ardrone.spinRight();
        }
        else if ((angle > 250 && angle < 290) && magnitude > 0.7)
        {
            System.out.println("turnleft ");
            ardrone.spinLeft();
        }

     }
    }

Answer 3

here you have angle and magnitude fields are used for comparision so create one method that accept these two as argument and pass the value to these method and return appropriate String to print

Answer 4

What about using a chain of responsibility? Behaviors that vary here could then be encapsulate in their own class.

In this case, these dedicated classes would act as listeners, as your teacher thought.

Each sequence of statements in your if/else blocks, would then be encapsulated in one specific object.

This would greatly help to keep following the Open/Closed Principle.

When more than one object may handle a request and the actual handler is not known in advance, you may use a Chain of Responsibility.

Some examples similar to yours (events from GUI) here:http://www.dcs.bbk.ac.uk/~oded/OODP13/Sessions/Session6/Chain.pdf