Java, what's wrong with this simple loop ?

grahamor

Hi,

This is an exam question from a past paper i'm going through. The question asks what is the intended purpose of the loop and what's wrong with the code.

I said the purpose of the loop is to find the first occurrence of a lowercase character. I am not clear what the problem with the loop is. I'm pretty sure it's to do with the boolean operator (!found) included in the for loop statement. But it seems to make sense to have it there so the loop doesnt continue when the first lowercase char is found. Should i use break to jump out of the loop instead ?

public class sampleQuestion {

	public static void main(String[] args){
	
		String text = "HElLo"; 
		
		boolean found = false;
		
		for(int i=0;!found && i<text.length();i++)
		{
			char ch = text.charAt(i);
			
			if(Character.isLowerCase(ch))
			{
				found = true;
				System.out.println(ch);
			}
			
		}
	}
}

Thanks for any tips

The Corinthian

Looks fine to and works fine for me, returning the letter 'l' correctly.

I'd use a break myself. Another approach would be to use the found boolean when testing for the lower case character:

if(!found && Character.isLowerCase(ch))

What problem are you finding?

grahamor

Thanks for checking,

I'm not finding any issues myself but the exam question says : explain what the program does and find out what is wrong with the code. Maybe it's a trick question.

robbiezero

Cant see anything wrong with it either.

Java Class names begin with an uppercase letter by convention. But its hardly that.

And it could maybe be slightly more efficient using a break.

Could do that code in any number of ways, with while loop, do-while etc, But that way is also correct.

robbiezero

grahamor wrote: »

Thanks for checking,

I'm not finding any issues myself but the exam question says : explain what the program does and find out what is wrong with the code. Maybe it's a trick question.

Unlikely to be a trick question I would think, It would be ridiculous.

grahamor

robbiezero wrote: »

Unlikely to be a trick question I would think, It would be ridiculous.

Thanks Robbie, yep i agree with what you said.

ectoraige

Seems ok to me, if text is null then the loop won't fire, and it terminates deterministically. I've certainly seen much worse code in the wild.

If the question is about loops then I suspect the use of the found flag is the problem, it's superflous when the break command can be used instead. Having to initiate the flag, test it, and update it introduces unneccessary complexity to the code, practices like that invariably lead to buggy code.

robbiezero

ectoraige wrote: »

Seems ok to me, if text is null then the loop won't fire, and it terminates deterministically. I've certainly seen much worse code in the wild.

If the question is about loops then I suspect the use of the found flag is the problem, it's superflous when the break command can be used instead. Having to initiate the flag, test it, and update it introduces unneccessary complexity to the code, practices like that invariably lead to buggy code.

If text is null an NPE will be fired when the text.length() is executed??

But in the above case that still is not wrong, as text cannot be null.

Ludo

Did the question ask what is "wrong" with the code or how it could be improved?

Hate those kinda questions as I cant see anything actually wrong with it either.

croo

Like everyone else I see nothing intrinsically wrong.

Maybe the questioners believe the loop should use techniques from latter Java versions...

for (char ch: text.toCharArray()) {
     if (Character.isLowerCase(ch))	{
	System.out.println(ch);
        break;
     }
}

I think this "for" structure was introduced in Java 5 [I said latter but I guess that is hardly new these days ].

But, like others, I would see that as personal preference (or coding standards within a project); the code itself, as is, seems okay to me and I would bet the compiled byte code that results is exactly the same.

grahamor

Ludo wrote: »

Did the question ask what is "wrong" with the code or how it could be improved?

Hate those kinda questions as I cant see anything actually wrong with it either.

The exact text is 'What is the intended purpose of this loop and what is wrong with the code'

I'm going to ask the lecturer at the revision class next week.

Thanks everyone for your input.

Clanket

Let us know what he says.

ectoraige

robbiezero wrote: »

If text is null an NPE will be fired when the text.length() is executed??

But in the above case that still is not wrong, as text cannot be null.

Yeah, I mis-typed (!), I should have said empty, not null.

There's an optimisation we've overlooked that might be the "problem".
Since the value of text is hardcoded, perhaps the code should read:

public class sampleQuestion {
	public static void main(String[] args){	
		String text = "HElLo"; 
		System.out.println("l");
	}
}

Of course, you've declared an unused variable, so a further optimisation would be:

public class sampleQuestion {
	public static void main(String[] args){	
		System.out.println("l");
	}
}

You may show this post to your lecturer if you think it'll help.