Add value to integer in do-while loop

Question

I'm having trouble having the do-while loop successfully change the integer to the value inside the if statement. It will exit the loop correctly if I enter 'y' or 'n', but the value of the integer will stay at 0.

I'm using substrings to allow the user to enter something like "yes" or "YEs", even "Y3$ir" and still equate to a 'y' to java.

Code:

import java.util.Scanner;
public class aTaskforAll
{
    public static void main (String [] args)
    {
        Scanner scan = new Scanner(System.in);
        String readAll;

        int readAllOption = 0;

        do {
            System.out.print("Do you want the words printed? (y/n) ");
            readAll = scan.nextLine();
            System.out.println(readAll.substring(0));

            if ((readAll.substring(0) == "y") || (readAll.substring(0) == "Y"))
                readAllOption = 1;
            else if ((readAll.substring(0) == "n") || (readAll.substring(0) == "N"))
                readAllOption = 2;
        }
        while (readAllOption != 0);

        System.out.println(readAllOption);    //Tester

        //Go on to do task in response to readAllOption = 1 or 2
    }
}

Bhesh Gurung · Accepted Answer · 2013-03-07 07:27:51Z

1

The String.substring(int) method does not do what you think it does.

Following is what the documentation says:

Returns a new string that is a substring of this string. The substring begins with the character at the specified index and extends to the end of this string.

So, readAll.substring(0) will give you a substring which will basically have all the characters from the original string.

The correct method for your use case is String.substring(int,int).

From the docs

Returns a new string that is a substring of this string. The substring begins at the specified beginIndex and extends to the character at index endIndex - 1. Thus the length of the substring is endIndex-beginIndex.

So, readAll.substring(0, 1) will give you a substring with only the first character.

Or it would be even better (cleaner) to use String.startsWith(String) method

if (readAll.startsWith("y") || readAll.startsWith("Y"))
    //...

Another problem is that are using == for object (strings in your case) equality check:

//...
if ((readAll.substring(0) == "y") || (readAll.substring(0) == "Y"))
//...
else if ((readAll.substring(0) == "n") || (readAll.substring(0) == "N"))
//...

Don't use ==, use .equals method instead for equality check. == is for identity check, which means just comparing the references.

Also, you should reset the value of readAllOption at the beginning of each iteration:

do {
    readAllOption = 0;

to exit the loop.

edited Mar 7, 2013 at 7:27

answered Mar 7, 2013 at 6:10

Bhesh Gurung

51.1k23 gold badges96 silver badges147 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

user949300 Over a year ago

-1 readAll.substring(0) does nothing. It returns the entire String.

Pejmon H. Over a year ago

I like using .startsWith for this situation. Thank you!

Jim Garrison Over a year ago

@BheshGurung Your original answer was in error (substring args). Rather than adding a correction as an edit, fix the original problem.

Jim Garrison Over a year ago

I mean you should edit your answer to make it look as if you had all the right information from the start. The objective is to make the answer as useful as possible to others who come searching later, as well as for the OP. Don't make others have to mentally keep track of the changes as they read.

Bhesh Gurung Over a year ago

@JimGarrison: Agreed. I have edited it. I hope it looks better this time. Let me know if I am stilling missing something.

|

Chris Dargis · Accepted Answer · 2013-03-07 06:12:44Z

1

Use .equals() instead of == when comparing strings.

Just getting the first character would also work (and it will be more efficient):

readAll.charAt(0) == 'y'

answered Mar 7, 2013 at 6:12

Chris Dargis

6,0634 gold badges42 silver badges66 bronze badges

6 Comments

user949300 Over a year ago

+1 for for charAt(). -1 for saying that it is more efficient. The issue is that readAllOption.subString(0) is outright wrong...

Chris Dargis Over a year ago

@user949300: Thanks for your input. How is it not more efficient? subString() creates a new string object whereas charAt() is a constant time operation returning a char.

user949300 Over a year ago

Unless he is calling this in a loop 100000 times a second, it won't matter. (But, basically you missed the bug in the concern for efficiency)

user949300 Over a year ago

(Though I may have mixed up your post with another...)

Chris Dargis Over a year ago

@user949300: I thought it was obvious from the use of parenthesis that it was more of a "hey just so you know this is more efficient" type thing. To your point, using charAt() in this instance doesn't offer a performance benefit. However I think this information can become useful to the OP down the road when his assignments get more complex. Stopping a complex algorithm to allocate memory instead of a constant time method call? Just thought it may become useful.

|

user949300 · Accepted Answer · 2013-03-07 06:18:17Z

1

First, your termination condition is backwards.

You want

do {
  stuff...
}
while (readAllOption  == 0)

As written, it will only exit the loop when readAllOption == 0. And you need to set it to 0 at the start of each loop.

Secondly, readAll.subString(0) returns the entire String. You want readAll.charAt(0), and compare that to the chars 'Y' or 'N'.

I have NO idea how this code was even remotely working for you.

edited Mar 7, 2013 at 6:18

answered Mar 7, 2013 at 6:11

user949300

15.7k7 gold badges39 silver badges69 bronze badges

Comments

Breavyn · Accepted Answer · 2013-03-07 06:19:08Z

1

With strings you need to use .equals("y") not == and you didn't have an ending index for the substring. so like this

if (readAll.substring(0, 1).equals("y")){}

As Bhesh Gurung said

Also, you should reset the value of readAllOption at the beginning of each iteration:

do { readAllOption = 0;

also as user user949300 said

Your termination condition is backwards.

You want

do { stuff... } while (readAllOption == 0) As written, it will only exit the loop when readAllOption == 0

edited Mar 7, 2013 at 6:19

answered Mar 7, 2013 at 6:12

Breavyn

2,25217 silver badges29 bronze badges

2 Comments

Breavyn Over a year ago

@Bhesh Gurung just beat me to it

Pejmon H. Over a year ago

Haha, thank you though! I did use your idea for ending the loop.

Pranav Kevadiya · Accepted Answer · 2013-03-07 06:33:53Z

Make your code like this. Maintaining integrity of it.

import java.util.Scanner;
public class aTaskforAll
  {
  public static void main (String [] args)
    {
    Scanner scan = new Scanner(System.in);
    String readAll;

    int readAllOption = 0;

    do {
        System.out.print("Do you want the words printed? (y/n) ");
        readAll = scan.nextLine();
        System.out.println(readAll.substring(0));

        if ("y".equals(readAll.substring(0,1).toLowerCase())
            readAllOption = 1;
        else if ("n".equals(readAll.substring(0,1).toLowerCase())
            readAllOption = 2;
        }
    while (readAllOption != 0);

    System.out.println(readAllOption);    //Tester

    //Go on to do task in response to readAllOption = 1 or 2
   }
}

Collectives™ on Stack Overflow

Add value to integer in do-while loop

5 Answers 5

6 Comments

6 Comments

Comments

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

6 Comments

6 Comments

Comments

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related