11 replies to this topic

[speachy_15]

And here i am, once again... asking for help... i had a program like this...but it sometimes produce the desired resullt...sometimes not... and i didn't use brackets because i saw a friend not use it... can anyone help????

 public static void main (String args[]) {


    	String [] array=new String [10];

    	int nItems=array.length;

    	int i,j;


    	for (i=0; i<array.length; i++){

    		String e=JOptionPane.showInputDialog ("Enter a string value:");

    		array[i]=e;

    	}



	String search=JOptionPane.showInputDialog ("Enter string search:");


    for (j=0; j<nItems; j++)

    		if (search.compareTo(array[j])>0)

    			break;

    		if (j==nItems)

    			JOptionPane.showMessageDialog (null, search+" not found!", "Case Study", JOptionPane.ERROR_MESSAGE);

    		else

    			JOptionPane.showMessageDialog (null, "Found "+search, "Case Study",JOptionPane.INFORMATION_MESSAGE);



		for(i=1 ; i<nItems ; i++){

            for(j=0 ; j<i ; j++){

                if(array[i].compareTo(array[j])<0){

                    String temp = array[i];

                    array[i] = array[j];

                    array[j] = temp;

                }

            }

        }


		String output="Array of characters in ascending order:"+"\n"+"\n";

        for(i=0 ; i<nItems ; i++)

           output+="  "+array[i]+" ";

           	JTextArea outputArea= new JTextArea ();

   			outputArea.setText(output);


   		JOptionPane.showMessageDialog (null, outputArea, "Case Study", JOptionPane.PLAIN_MESSAGE);




		for(i=1 ; i<nItems ; i++){

            for(j=0 ; j<i ; j++){

                if(array[i].compareTo(array[j])>0){

                    String temp = array[i];

                    array[i] = array[j];

                    array[j] = temp;

                }

            }

        }


		String output2="Array of characters in descending order:"+"\n"+"\n";

        for(i=0 ; i<nItems ; i++)

           output2+="  "+array[i]+" ";

           	JTextArea outputArea2= new JTextArea ();

   			outputArea2.setText(output2);


   		JOptionPane.showMessageDialog (null, outputArea2, "Case Study", JOptionPane.PLAIN_MESSAGE);



		String del=JOptionPane.showInputDialog ("Enter string to delete:");


	JOptionPane.showMessageDialog (null,"\n"+"Delete element "+del,"Case Study", JOptionPane.INFORMATION_MESSAGE);


    	for (j=0; j<nItems; j++)

    			if (del.compareTo(array[j])>0)

    				break;

    			for (int k=j; k<nItems-1; k++)

    				array[j]=array[k+1];

    				nItems--;

    				

		String output3="Array after deletion of element "+del;


   		for (j = 0; j <nItems; j ++)

   				output3 +="\n"+array[j]+ "  ";


   				JTextArea outputArea3= new JTextArea ();

   				outputArea3.setText(output3);

   			    JOptionPane.showMessageDialog (null, outputArea3, "Case Study", JOptionPane.PLAIN_MESSAGE);

   			    System.exit(0);

    	}

    }

as you can see... its supposed to make you enter 10 strings... then you search for an item... then it sorts the elements... then you delete an item....

thank you very, very much!!!!

[lethalwire]

You might be missing a few { braces } for some of your "for" loops. For instance:

    for (j=0; j<nItems; j++) [COLOR="Red"]{[/COLOR]

            if (search.compareTo(array[j])>0)

                break;

            if (j==nItems)

                JOptionPane.showMessageDialog (null, search+" not found!", "Case Study", JOptionPane.ERROR_MESSAGE);

            else

                JOptionPane.showMessageDialog (null, "Found "+search, "Case Study",JOptionPane.INFORMATION_MESSAGE); 

[COLOR="Red"]}[/COLOR]

This previous portion of code also seems suspicious to me.
Here is why:

if ( search.compareTo( array[j]) > 0 )

So you test if the return value of compareTo is greater than zero. Good
But do you test to see if the return value of compareTo is less than zero?

If one of these passes the test, then why do you call "break"?

What if your String array looks like this
String[] array = {a, b, c, d, e, f, g, h, i, j}; // I omitted quotes.
let search = z;

So the first time we loop through i = 0;

if( z.compareTo( a ) > 0 ) // this will return true in this case

    break;

This automatically stops your loop from continuing! You might be more interested in using the "continue" keyword instead of the "break" keyword.

You also have ( if j == nItems )
This will NEVER evaluate to true. the only time j == nItems is when the loop is finished.
If you want to know if search.compareTo( array[i] ) ever matches, then you'll need to use a variable to inform you of this.

I'll write out some pseudocode.

found = false;

search = getUserInput;

for each ( value in String[] array ) {

    if( search.compareTo( value ) < 0 )

        continue;  [COLOR="Red"]// we'll continue searching[/COLOR]

    else if( search.compareTo( value ) > 0 )

        continue; [COLOR="Red"]// we'll continue searching[/COLOR]

    else {

        found = true;

        break; [COLOR="Red"]// now we can successfully terminate this loop[/COLOR]

    }

}

[COLOR="Red"]// after the loop is done[/COLOR]

if( found )

    print ( we found it );

else

    print( we didn't find it );

Does this solve your problem?

[BlaineSch]

lethalwire said:

You might be missing a few { braces } for some of your "for" loops.

A few? Don't be so modest :c-lol:

In all seriousness, just because you -can- get away without using brackets in some places does not mean you -should-. It creates sloppy code which can be hard to debug. On the same note, don't forget to comment code! What may seem obvious to you, may not to fresh eyes.

[wim DC]

I always use the { and } even if it's a 1-liner in it and could be written without.

[mnirahd]

Hi,

I think you need to use braces here

    for (j=0; j<nItems; j++)

            if (search.compareTo(array[j])>0)

                break;

            if (j==nItems)

                JOptionPane.showMessageDialog (null, search+" not found!", "Case Study", JOptionPane.ERROR_MESSAGE);

            else

                JOptionPane.showMessageDialog (null, "Found "+search, "Case Study",JOptionPane.INFORMATION_MESSAGE);

I hope this helps!

Munir

[Liars_paradox]

nm.

[Liars_paradox]

lethalwire said:

You also have ( if j == nItems )
This will NEVER evaluate to true. the only time j == nItems is when the loop is finished.
If you want to know if search.compareTo( array[i] ) ever matches, then you'll need to use a variable to inform you of this.

Actually you're wrong, because that j is declared outside of the for-loop. Since, j is accessible outside of the for-loop, j will eventually be as large as nItems if the loop executes without ever breaking.

Also, if you'll notice, he didn't have the brackets surrounding all of the conditional statements. This means that the first if statement is evaluated within the for-loop, but the preceding if-else statement is evaluated outside of the loop.

Your error is likely due to the confusion from the OP's use of tabs before the if-else statement which makes it look like it's apart of the for-loop, or from your lack of understanding of how a for-loop works.

When a for-loop executes, the loop will execute a total of 'n' times, with 'n' being the sentinel value for the loop. All of the statements within the loop are executed a total of n-1 times. For the following for loop:

int j;

int count = 0;

for(j = 0; j<n; j++)

count++;

The loop continues to increment j until its value is equal to n, thus making the conditional statement "j<n" equal to false. On the other hand, the "count" variable will equal n-1 when the loop finishes.

I think that declaring j outside of the first for-loop, like the OP had done in the first place, is a great way to test if the search string is even stored in the loop or not. If it is in the loop, then j will not be equal to the size of the array. However, if the search item isn't in the loop then that means the program would've looked through the entire loop and found that it wasn't there. Thus, making j equal to the size of the array.

lethalwire said:

I'll write out some pseudocode.

found = false;

search = getUserInput;

for each ( value in String[] array ) {

    if( search.compareTo( value ) < 0 )

        continue;  [COLOR="Red"]// we'll continue searching[/COLOR]

    else if( search.compareTo( value ) > 0 )

        continue; [COLOR="Red"]// we'll continue searching[/COLOR]

    else {

        found = true;

        break; [COLOR="Red"]// now we can successfully terminate this loop[/COLOR]

    }

}

[COLOR="Red"]// after the loop is done[/COLOR]

if( found )

    print ( we found it );

else

    print( we didn't find it );

Does this solve your problem?

You see, the problem with your code is that you make the code too complicating and cause the program to execute more statements than it needs to. If the OP is just trying to see if his search string is inside of his collection of strings, then testing the size of j after the loop executes is just fine.

Your code, in a worst-case scenario, would execute a total of 3 statements, multiplied by the size of the array minus one. Or, for n = array_size - 1, would have a complexity of Ο(3n). Whereas, the OP's code has a complexity of just Ο(n).

Edited by Liars_paradox, 19 December 2010 - 09:06 PM.

[Liars_paradox]

speachy_15 said:

for (j=0; j<nItems; j++)

	if (search.compareTo(array[j])>0)

		break;

	if (j==nItems)

		JOptionPane.showMessageDialog (null, search+" not found!", "Case Study", JOptionPane.ERROR_MESSAGE);

	else

		JOptionPane.showMessageDialog (null, "Found "+search, "Case Study",JOptionPane.INFORMATION_MESSAGE);

Assuming that you're trying to test to see if the search string is stored within your loop with this statement, then you probably should change your if statement to the following:

for (j=0; j<nItems; j++)

	if (search.compareTo(array[j])!=0)

		break;

if (j==nItems)

	JOptionPane.showMessageDialog (null, search+" not found!", "Case Study", JOptionPane.ERROR_MESSAGE);

else

	JOptionPane.showMessageDialog (null, "Found "+search, "Case Study",JOptionPane.INFORMATION_MESSAGE);

 

Or, for those who are sticklers over braces, then you can use the following:

for (j=0; j<nItems; j++){

	if (search.compareTo(array[j])!=0){

		break;

	}

}

if (j==nItems){

	JOptionPane.showMessageDialog (null, search+" not found!", "Case Study", JOptionPane.ERROR_MESSAGE);

}

else{

	JOptionPane.showMessageDialog (null, "Found "+search, "Case Study",JOptionPane.INFORMATION_MESSAGE);

}

 

Here is my reason for doing this, according to Java™ Platform, Standard Edition 7 API Specification, for the java.lang.String class's documentation, the "compareTo" method:

Quote

Compares two strings lexicographically... The result is zero if the strings are equal; compareTo returns 0 exactly when the equals(Object) method would return true.

source: Java Platform SE 7 b120

So, if you're wanting to break out of the loop whenever you've found your search item, then break out of the loop when the compareTo method returns 0 instead of a positive integer.

Edited by Liars_paradox, 20 December 2010 - 09:52 AM.

[Liars_paradox]

wim DC said:

I always use the { and } even if it's a 1-liner in it and could be written without.

Right, but not everyone does because it takes up more space and can look ugly to some. If you were to work in "software support" or as an "integration specialist", then you'd probably have to get used to things like people not using braces around in all the places that they should.

[lethalwire]

I was supposing the case that he honestly forgot to add the braces for the loop.
I could have also supposed your way too. Either way, will work.

Knowing that newcomers honestly forgot simple things like braces, is the reason I assumed this.

Quote

or from your lack of understanding of how a for-loop works.

If I'm trying to provide an answer to the OP, I'd hope I know a thing or two about how for-loops work. ;)

Quote

You see, the problem with your code is that you make the code too complicating and cause the program to execute more statements than it needs to. If the OP is just trying to see if his search string is inside of his collection of strings, then testing the size of j after the loop executes is just fine.

They code may be more complicated in the sense that it takes longer to finish, but it is easier to understand. That is the point I was trying to reach to the OP.
I don't want the OP to read my sample and be confused, but rather give him a general idea of how the algorithm could have been written.

From this example, yes he can easily work-out and understand that these statements could be condensed and optimized to make the algorithm more efficient.

Therefore, I agree that the way you've written it is correct, it's just not the path I chose for the OP.

Anyways, the following code you provided seems incorrect to me.

for (j=0; j<nItems; j++)

    if (search.compareTo(array[j])!=0)[COLOR="SeaGreen"]// why do we stop the search IF the strings didn't match?[/COLOR]

        break; [COLOR="SeaGreen"]// shouldn't we stop IF the strings match?[/COLOR]

if (j==nItems)

    JOptionPane.showMessageDialog (null, search+" not found!", "Case Study", JOptionPane.ERROR_MESSAGE);

else

    JOptionPane.showMessageDialog (null, "Found "+search, "Case Study",JOptionPane.INFORMATION_MESSAGE);  

**Edit, now that I look at it, the code I posted is not only correct, but also runs in O(n), not O(3n).
My Code:

		for(int j = 0; j < array.length; ++j ) {

		    if( search.compareTo( array[j] ) < 0 )

		        continue;  // we'll continue searching BECAUSE STRING NOT MATCHED

		    else if( search.compareTo( array[j] ) > 0 )

		        continue; // we'll continue searching BECAUSE STRING NOT MATCHED

		    else {

                        found = true;

		        break; // now we can successfully terminate this loop BECAUSE THE STRING MATCHED

		    }

		}

[Liars_paradox]

lethalwire said:

Anyways, the following code you provided seems incorrect to me.

for (j=0; j<nItems; j++)

    if (search.compareTo(array[j])!=0)[COLOR="SeaGreen"]// why do we stop the search IF the strings didn't match?[/COLOR]

        break; [COLOR="SeaGreen"]// shouldn't we stop IF the strings match?[/COLOR]

if (j==nItems)

    JOptionPane.showMessageDialog (null, search+" not found!", "Case Study", JOptionPane.ERROR_MESSAGE);

else

    JOptionPane.showMessageDialog (null, "Found "+search, "Case Study",JOptionPane.INFORMATION_MESSAGE);  

Yeah, my bad. You're right, I don't know why I had "!=" in there. lol.

[speachy_15]

Thanks guys... sorry i wasn't able to post an immediate reply.... just got my computer fresh from the repairman..
i followed lethalwire's advice.... and it got through... and no, i think i know mostly how for loops work.. its just i was experimenting on what would happen if i didn't put in braces...
and i really, really thank everybody who replied here.... really.. thank you!!!!