[Duta]

I found this in some Java source code...

public void input()
    {
        if(timer>= 5)
        {
            if(Greenfoot.isKeyDown("a")){actWord = actWord.append("a") ;stars =stars.append("*");timer = 0;}if(Greenfoot.isKeyDown("b")){actWord=actWord.append("b");stars = stars.append("*");timer = 0;}
            if(Greenfoot.isKeyDown("c")){actWord = actWord.append("C");stars = stars.append("*");timer = 0;}if(Greenfoot.isKeyDown("d")){actWord = actWord.append("d");stars = stars.append("*");timer = 0;}
            if(Greenfoot.isKeyDown("e")){actWord = actWord.append("e");stars = stars.append("*");timer = 0;}if(Greenfoot.isKeyDown("f")){actWord = actWord.append("f");stars =stars.append("*");timer = 0;} 
            if(Greenfoot.isKeyDown("g")){actWord = actWord.append("g");stars = stars.append("*");timer = 0;}if(Greenfoot.isKeyDown("h")){actWord = actWord.append("h");stars = stars.append("*");timer = 0;}
            if(Greenfoot.isKeyDown("i")){actWord = actWord.append("i");stars = stars.append("*");timer = 0;}if(Greenfoot.isKeyDown("j")){actWord = actWord.append("j");stars = stars.append("*");timer = 0;}
            if(Greenfoot.isKeyDown("k")){actWord = actWord.append("k");stars =stars.append("*");timer = 0;}if(Greenfoot.isKeyDown("l")){actWord =  actWord.append("l");stars =stars.append("*");timer = 0;}
            if(Greenfoot.isKeyDown("m")){actWord = actWord.append("m");stars = stars.append("*");timer = 0;}if(Greenfoot.isKeyDown("n")){actWord = actWord.append("n");stars = stars.append("*");timer = 0;}
            if(Greenfoot.isKeyDown("o")){actWord = actWord.append("o");stars = stars.append("*");timer = 0;}if(Greenfoot.isKeyDown("p")){actWord = actWord.append("p");stars = stars.append("*");timer = 0;}
            if(Greenfoot.isKeyDown("q")){actWord = actWord.append("q");stars = stars.append("*");timer = 0;}if(Greenfoot.isKeyDown("r")){actWord = actWord.append("r");stars = stars.append("*");timer = 0;}
            if(Greenfoot.isKeyDown("s")){actWord = actWord.append("s");stars = stars.append("*");timer = 0;}if(Greenfoot.isKeyDown("t")){actWord = actWord.append("t");stars = stars.append("*");timer = 0;}
            if(Greenfoot.isKeyDown("u")){actWord = actWord.append("u");stars = stars.append("*");timer = 0;}if(Greenfoot.isKeyDown("v")){actWord = actWord.append("v");stars = stars.append("*");timer = 0;}
            if(Greenfoot.isKeyDown("w")){actWord = actWord.append("w");stars = stars.append("*");timer = 0;}if(Greenfoot.isKeyDown("x")){actWord = actWord.append("x");stars = stars.append("*");timer = 0;}
            if(Greenfoot.isKeyDown("y")){actWord = actWord.append("y");stars =stars.append("*");timer = 0;}if(Greenfoot.isKeyDown("z")){actWord = actWord.append("z");stars = stars.append("*");timer = 0;}
            if(Greenfoot.isKeyDown("backspace"))if(actWord.length() > 0){actWord.deleteCharAt(actWord.length() - 1);stars.deleteCharAt(stars.length() - 1);;timer = 0;}
            if(Greenfoot.isKeyDown("space")){actWord = actWord.append(" ");stars = stars.append("*");timer = 0;}
        }
        else 
            timer ++;

    }

Repeated code, hideous formatting, empty statements (see line 18's double semicolon) - whatever does it for you, this method has it!

[MentalFloss]

Well, yes it is bad but I noticed in your code you had a peculiar variable name of Greenfoot. I did some research into this and it would appear to be a toolkit to create applications in what seems like a highly generated fashion.

Now, I can imagine the case where someone were to be using this particular interface and adding some kind of input box to their project with some configuration details about its behavior in a GUI. From that information, the Greenfoot application generates the code required.

Generated code is rarely pretty or editable especially in regards to GUI elements. So, this code is perfectly reasonable for how it was created.

If, however, this code was created by hand in an effort to provide the functionality, I found a link that might be adequate to your refactoring needs.

Please review Greenfoot discussion on isKeyDown.

I hope this helps elucidate things for you.

Good day.

This post has been edited by MentalFloss: 29 June 2012 - 05:38 PM

[Duta]

MentalFloss, on 29 June 2012 - 05:37 PM, said:

Well, yes it is bad but I noticed in your code you had a peculiar variable name of Greenfoot. I did some research into this and it would appear to be a toolkit to create applications in what seems like a highly generated fashion.

Now, I can imagine the case where someone were to be using this particular interface and adding some kind of input box to their project with some configuration details about its behavior in a GUI. From that information, the Greenfoot application generates the code required.

Generated code is rarely pretty or editable especially in regards to GUI elements. So, this code is perfectly reasonable for how it was created.

If, however, this code was created by hand in an effort to provide the functionality, I found a link that might be adequate to your refactoring needs.

Please review Greenfoot discussion on isKeyDown.

I hope this helps elucidate things for you.

Good day.

Firstly, I know about Greenfoot - It was what got me into Java and coding in the first place. To clarify, Greenfoot doesn't generate any code whatsoever. And I don't need to refactor it (I'm not using the code)..

[Skydiver]

Although there are reasons to have loops, there are also reasons to unroll loops. Check out the one of the recent C# challenges in this forum, or some crypto code for computing hashes, CRCs, or new cyphertext where people deliberately unroll loops to get performance gains.

But yes, in general, loops are good because it makes code more legible.

[Duta]

Indeed, I mean, something like the following (which could be made better, for example programmatically generating the letters array, however this is just for a point) is just so clearer

public void input() {
	if(timer >= 5) {
		String[] letters =
			{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m",
			 "n", "o", "p", "q", "r", "s", "t", "u", "v", "w", "x", "y", "z"};
		for(String letter : letters) {
			if(Greenfoot.isKeyDown(letter)) {
				actWord = actWord.append(letter);
				stars = stars.append("*");
				timer = 0;
			}
		}
		if(Greenfoot.isKeyDown("backspace")
		&& actWord.length() > 0) {
			actWord.deleteCharAt(actWord.length() - 1);
			stars.deleteCharAt(stars.length() - 1);
			timer = 0;
		}
		if(Greenfoot.isKeyDown("space")) {
			actWord = actWord.append(" ");
			stars = stars.append("*");
			timer = 0;
		}
	} else {
		timer++;
	}
}

[MsParnel]

Make a method to not repeat the if's 3 times