0

Hi i made a small shapes game and my if statements are starting to get out of hand, so i made a simpler test scenario game to try to simplify the code but im not as experienced with java and android as id like to be, so after trying a few things i thought id ask the question, how do i make this code smaller? At the moment i am using onTouchListeners and onDragListeners for a few shapes, atm 3 coloured shapes and 3 blank 'empty shapes' and when they are connected by putting one on top of another the empty shape becomes coloured.... very simple. but takes a ton of code heres what i had

@Override
public boolean onTouch(View v, MotionEvent e) {
    if (e.getAction() == MotionEvent.ACTION_DOWN) {
        DragShadowBuilder shadowBuilder = new View.DragShadowBuilder(v);
        v.startDrag(null, shadowBuilder, v, 0);
        sp.play(dragSound, 1, 1, 0, 0, 1);
        return true;
    } else {
        return false;
    }
}

//WHEN DRAGGED AND DROPPED

@Override
public boolean onDrag(View v, DragEvent e) {

    if (e.getAction()==DragEvent.ACTION_DROP) {
        View view = (View) e.getLocalState();

        //IF THEY MATCH

        if(view.getId()==R.id.squareImage && v.getId()==R.id.squareImage1)
        {
            ViewGroup from = (ViewGroup) view.getParent();
            ViewGroup to = (ViewGroup) findViewById(R.id.layout2);
            View congrats = findViewById(R.id.top_layout);
            ViewGroup two = (ViewGroup) findViewById(R.id.layout3);

            from.removeView(view);
            v.setBackgroundResource(R.drawable.dragsquare);
            sp.play(dropSound, 1, 1, 0, 0, 1);
            sb.setVisibility(View.VISIBLE);

            imageView = (ImageView)findViewById(R.id.imageView);
            imageView.setVisibility(View.VISIBLE);

            if (to.getChildCount()< 1 && two.getChildCount()< 1)

            {
                congrats.setVisibility(View.VISIBLE);
                imageView.setBackgroundResource(R.drawable.congrats);
                sun=(AnimationDrawable)imageView.getBackground();
                sun.start();

                sp.play(tada, 1, 1, 0, 0, 1);
                congrats.setOnClickListener(new View.OnClickListener() {
                    @Override
                    public void onClick(View v) {
                      View congrats
(RelativeLayout)findViewById(R.id.top_layout);
                        congrats.setVisibility(View.INVISIBLE);

                    }
                });

            }

//2 square balloons floating


            sb = (ImageView)findViewById(R.id.squareballoon);
            sb.setVisibility(View.VISIBLE);

            sb2 = (ImageView)findViewById(R.id.squareballoon2);
            sb2.setVisibility(View.VISIBLE);

            sp.play(inflate, 1, 1, 0, 0, 1);

            ObjectAnimator sqbalAnim3=
 ObjectAnimator.ofFloat(sb2,"x",-500,500);
            sqbalAnim3.setDuration(700);
            sqbalAnim3.setRepeatCount(20);
            sqbalAnim3.setRepeatMode(ValueAnimator.REVERSE);

            ObjectAnimator sqbalAnim =   
ObjectAnimator.ofFloat(sb2,"y",1800,-1800);
            sqbalAnim.setDuration(3000);
            sqbalAnim.setRepeatMode(ValueAnimator.RESTART);

            AnimatorSet animSetXY = new AnimatorSet();
            animSetXY.addListener(new AnimatorListenerAdapter() {
                @Override
                public void onAnimationEnd(Animator animation) {
                    super.onAnimationEnd(animation);
                    sb2.setVisibility(View.GONE);
                }
            });
            animSetXY.playTogether(sqbalAnim, sqbalAnim3);
            animSetXY.setStartDelay(20);
            animSetXY.start();


            ObjectAnimator sqbal2Anim =
 ObjectAnimator.ofFloat(findViewById(R.id.squareballoon2),"y",-450);
            sqbal2Anim.setDuration(3000);
            sqbal2Anim.setRepeatMode(ValueAnimator.RESTART);

            ObjectAnimator sqbalAnim4 =
 ObjectAnimator.ofFloat(findViewById(R.id.squareballoon2),"x",650,750);
            sqbalAnim4.setStartDelay(20);
            sqbalAnim4.setDuration(300);
            sqbalAnim4.setRepeatCount(6);
            sqbalAnim4.setRepeatMode(ValueAnimator.REVERSE);

            AnimatorSet animSetXY2 = new AnimatorSet();
            animSetXY2.playTogether(sqbal2Anim,sqbalAnim4);
            animSetXY2.start();

            return true;}

        //end of square balloons


        else if(view.getId()==R.id.circleImage &&
 v.getId()==R.id.circleImage1){

            ViewGroup from = (ViewGroup) view.getParent();
            ViewGroup to = (ViewGroup) findViewById(R.id.layout2);
            View congrats = findViewById(R.id.top_layout);

            from.removeView(view);
            v.setBackgroundResource(R.drawable.dragcircle);
            sp.play(dropSound, 1, 1, 0, 0, 1);
            cb = (ImageView)findViewById(R.id.circleballoon);
            cb.setVisibility(View.VISIBLE);

            imageView.setVisibility(View.VISIBLE);
            ViewGroup two = (ViewGroup) findViewById(R.id.layout3);

            if (to.getChildCount()< 1 && two.getChildCount()< 1)

            {
                congrats.setVisibility(View.VISIBLE);
                imageView.setBackgroundResource(R.drawable.congrats);
                sun=(AnimationDrawable)imageView.getBackground();
                sun.start();

                sp.play(tada, 1, 1, 0, 0, 1);
                congrats.setOnClickListener(new View.OnClickListener() {
                    @Override
                    public void onClick(View v) {
                        View congrats
 (RelativeLayout)findViewById(R.id.top_layout);
                        congrats.setVisibility(View.INVISIBLE);

                    }
                });

            }

 //circle balloons floating



            cb = (ImageView)findViewById(R.id.circleballoon);
            cb.setVisibility(View.VISIBLE);
            cb2 = (ImageView)findViewById(R.id.circleballoon2);
            cb2.setVisibility(View.VISIBLE);

            sp.play(inflate, 1, 1, 0, 0, 1);

            ObjectAnimator sqbalAnim3 =
ObjectAnimator.ofFloat(cb,"x",-500,500);
            sqbalAnim3.setDuration(700);
            sqbalAnim3.setRepeatCount(20);
            sqbalAnim3.setRepeatMode(ValueAnimator.REVERSE);

            ObjectAnimator sqbalAnim =
ObjectAnimator.ofFloat(cb,"y",1800,-1800);
            sqbalAnim.setDuration(3000);
            sqbalAnim.setRepeatMode(ValueAnimator.RESTART);

            AnimatorSet animSetXY = new AnimatorSet();
            animSetXY.addListener(new AnimatorListenerAdapter() {
                @Override
                public void onAnimationEnd(Animator animation) {
                    super.onAnimationEnd(animation);
                    cb.setVisibility(View.GONE);
                }
            });
            animSetXY.playTogether(sqbalAnim, sqbalAnim3);
            animSetXY.setStartDelay(20);
            animSetXY.start();


            ObjectAnimator sqbal2Anim =
ObjectAnimator.ofFloat(findViewById(R.id.squareballoon2),"y",-450);
            sqbal2Anim.setDuration(3000);
            sqbal2Anim.setRepeatMode(ValueAnimator.RESTART);

            ObjectAnimator sqbalAnim4 =
ObjectAnimator.ofFloat(findViewById(R.id.squareballoon2),"x",650,750);
            sqbalAnim4.setStartDelay(20);
            sqbalAnim4.setDuration(300);
            sqbalAnim4.setRepeatCount(6);
            sqbalAnim4.setRepeatMode(ValueAnimator.REVERSE);

            AnimatorSet animSetXY2 = new AnimatorSet();
            animSetXY2.playTogether(sqbal2Anim,sqbalAnim4);
            animSetXY2.start();

            return true;}




        } else if(view.getId()==R.id.triangleImage && 
 v.getId()==R.id.triangleImage1){
            ViewGroup from = (ViewGroup) view.getParent();
            ViewGroup to = (ViewGroup) findViewById(R.id.layout2);
            View congrats = findViewById(R.id.top_layout);
            from.removeView(view);
            v.setBackgroundResource(R.drawable.dragtriangle);
            sp.play(dropSound, 1, 1, 0, 0, 1);
            tb = (ImageView)findViewById(R.id.triballoon);
            tb.setVisibility(View.VISIBLE);
            imageView.setVisibility(View.VISIBLE);
            ViewGroup two = (ViewGroup) findViewById(R.id.layout3);

            if (to.getChildCount()< 1 && two.getChildCount()< 1)

            {
                congrats.setVisibility(View.VISIBLE);
                imageView.setBackgroundResource(R.drawable.congrats);
                sun=(AnimationDrawable)imageView.getBackground();
                sun.start();

                sp.play(tada, 1, 1, 0, 0, 1);
                congrats.setOnClickListener(new View.OnClickListener() {
                    @Override
                    public void onClick(View v) {
                        View congrats = findViewById(R.id.top_layout);
                        congrats.setVisibility(View.INVISIBLE);

                    }
                });

            }


            AnimatorSet sunSet = (AnimatorSet)
 AnimatorInflater.loadAnimator(this, R.animator.float1);
            sunSet.setTarget(tb);
            sunSet.start();

            tb = (ImageView)findViewById(R.id.triballoon);

            AnimatorSet sunnySet = (AnimatorSet)
 AnimatorInflater.loadAnimator(this, R.animator.float2);
            sunnySet.setTarget(tb);
            sunnySet.start();

            ImageView tb2 = (ImageView)findViewById(R.id.triballoon2);
            tb2.setVisibility(View.VISIBLE);

            AnimatorSet sunSet1 = (AnimatorSet)
  AnimatorInflater.loadAnimator(this, R.animator.float3);
            sunSet1.setTarget(tb2);
            sunSet1.start();

            tb2 = (ImageView)findViewById(R.id.triballoon2);
            tb2.setVisibility(View.VISIBLE);

            AnimatorSet sunnySet1 = (AnimatorSet)   
    AnimatorInflater.loadAnimator(this, R.animator.float2);
            sunnySet1.setTarget(tb);
            sunnySet1.start();
            sp.play(inflate, 1, 1, 0, 0, 1);

            return true;

This was my original what i want to know is, is there a way to use the OR operator to put them all in the same statement yet still give different results for each shape something like this

@Override
public boolean onDrag(View v, DragEvent e) {

    if (e.getAction() == DragEvent.ACTION_DROP) {
        View view = (View) e.getLocalState();

        //IF THEY MATCH

        if (view.getId() == R.id.squareshape && v.getId() ==
R.id.emptysquare || view.getId() == R.id.circleshape && v.getId() ==
R.id.emptycircle|| view.getId() == R.id.trishape && v.getId() ==
R.id.emptytri ) {

         //view.getId().(v.getId());  view.setBackgroundResource(v)

            mt_sq.setImageResource(R.drawable.dragsquare);
        }else{
            //do nothing
        }

    }
    return true;
}

the comment in the middle is what i want to achieve //view.getId().(v.getId()); view.setBackgroundResource(v) but obviouslt this gives me errors can anyone offer a solution or do i just need to keep trucking on my original? any and all suggestions welcome

martinseal1987
  • 1,862
  • 8
  • 44
  • 77
  • 1
    Use ButterKnife library from Jake Wharton, so you dont use findViewById constantly. – Kevin Crain Feb 10 '15 at 10:37
  • Butter knife looks great i read through the website but I wouldn't know how to implement this, but for more experienced coders it would be the answer so I'll mark this as the answer soon unless somebody weighs in with a simpler solution thank you @Kevin Crain – martinseal1987 Feb 10 '15 at 10:56
  • Its not for experienced programmers, its easy to implement and use. :) – Kevin Crain Feb 10 '15 at 11:09
  • I know, but looks like I need to edit gradle files in android studio to use it and that never goes well for me lol still haven't managed to get Facebook sdk working with studio – martinseal1987 Feb 10 '15 at 11:22
  • http://gradleplease.appspot.com, you can use this link to find libraries, step one add library to project with gradle compile, step 2 if you using an activity or a fragment will differ a little, for activity in the onCreate use ButterKnife.inject(this), for a fragment in the onCreateView use ButterKnife.inject(this, rootView), Step 3 will be using InjectView annotation for all the views you want to inject, all look up injecting invents so you can mark a method with something like OnClick annotation instead of using setOnClickListener – Kevin Crain Feb 10 '15 at 12:11

1 Answers1

0

A similar question was asked on another SE site, the core of which is very similar to your question -- avoid bloat in game programming/software development in general. My answer to it can be found here, but you should read the modified answer below, as it's formatted to tackle your question in a general way.

Why are if statements evil:

Groo posted a fantastic answer to this question in a StackOverflow thread several years ago, which you can find here, which boils down to readability and future proofing is an enemy of the if statements.

That's not to say that you should always avoid if statements though! They have their uses, in simple, one off checks.

Writing clean game code:

Imagine we have a statement, which draws a sprite our render function:

if (powerUpActive){

    draw(shieldSprite);

}

After a couple of weeks of development, the render function becomes bloated and hard to traverse. To fix this, we should move the above logic ( and all similar logic ) inside a function, in the class PowerUp like so:

class PowerUps {
    public void Check() {
        if (powerUpActive){

            draw(shieldSprite);

        }
    }
}

and just call

powerUp.Check; 

from render.

To future proof this, I would also even change powerUpActive to something even more specific like a class Shield, inside class PowerUp and then check it's activity with shield.enable() like so:

class PowerUps {
    class Shield {
        private boolean active = false;

        public void enable() {
            active = true;
        }

        public void disable() {
            active = false;
        }
    }
    public void Check() {
        if (shield.active){

            draw(shieldSprite);

        }
    }
}

The purpose of this is two-fold -- you can enable/disable power-ups from a game event easily, by calling powerUp.shield.enable() and draw it without having to worry too much about backtracking.

This may seem like a lot of work for one shield power-up, but it's a question of future-proofing your code and saving yourself hours, upon hours of hard work later on, when you would eventually want to expand.

Just think think about how pretty your render function will look! And how easy it will be to expand your game without worrying if you've broken something! I don't know about you, but I'm excited already! :3

NB: The examples I provided are just to elaborate on a point. In the real world you would probably want to use inheritance more actively and organize things even more, like keeping a reference to the shieldSprite inside the Shield class.

Community
  • 1
  • 1
Sipty
  • 1,159
  • 1
  • 10
  • 18
  • 1
    I think what I'm wanting is much simpler (and maybe isn't possible) than what your suggesting at, for instance this game will never expand its a mere distraction in a much bigger app hence my wanting for a simpler amount of code but your answer says alot, and much of what you said will help me so I will mark yours as correct thank you very much – martinseal1987 Feb 10 '15 at 11:21
  • If you're not going to expand the app farther, there is no point of wasting time on optimizing it -- you've learned what you needed from it and should move on. Especially if you just wanted to shrink it's size just for the sake of less code. Best of luck in the future! – Sipty Feb 10 '15 at 11:43
  • 1
    Thanks Sipty your right and that's what I've taken away from this, but its now done and serves its purpose, thanks for all the replies – martinseal1987 Feb 11 '15 at 00:49