String not populating properly

I am writing a program that is going to read a string from a file, and then remove anything that isn't 1-9 or A-Z or a-z. The A-Z values need to become lowercase. Everything seems to run fine, I have no errors, however my output is messed up. It seems to skip certain characters for no reason whatsoever. I've looked at it and tweaked it but nothing works. Can't figure out why it is randomly skipping certain characters because I believe my if statements are correct. Here is the code:

    String dataIn;
    int temp;
    String newstring= "";
    BufferedReader file = new BufferedReader(new FileReader("palDataIn.txt")); 
    while((dataIn=file.readLine())!=null)
      {
        newstring="";
        for(int i=0;i<dataIn.length();i++)
          {
            temp=(int)dataIn.charAt(i);
            if(temp>46&&temp<58)
              {
                newstring=newstring+dataIn.charAt(i);
              }
            if(temp>96&&temp<123)
              {
                newstring=newstring+dataIn.charAt(i);
              }
            if(temp>64&&temp<91)
              {
                newstring=newstring+Character.toLowerCase(dataIn.charAt(i));
              }
            i++;
           }
        System.out.println(newstring);
      }

So to give you an example, the first string I read in is :

A sample line this is.

The output after my program runs through it is this:

asmlietis

So it is reading the A making it lowercase, skips the space like it is suppose to, reads the s in, but then for some reason skips the "a" and the "m" and goes to the "p".

Jon Skeet
people
quotationmark

You're incrementing i in the each of the blocks as well as in the main loop "header". Indeed, because you've got one i++; in an else statement for the last if statement, you're sometimes incrementing i twice during the loop.

Just get rid of all the i++; statements other than the one in the for statement declaration. For example:

newstring="";
for(int i=0;i<dataIn.length();i++)
{
    temp=(int)dataIn.charAt(i);
    if(temp>46&&temp<58)
    {
        newstring=newstring+dataIn.charAt(i);
    }
    if(temp>96&&temp<123)
    {
        newstring=newstring+dataIn.charAt(i);
    }
    if(temp>64&&temp<91)
    {
        newstring=newstring+Character.toLowerCase(dataIn.charAt(i));
    }
}

I wouldn't stop editing there though. I'd also:

  • Use a char instead of an int as the local variable for the current character you're looking at
  • Use character literals for comparisons, to make it much clearer what's going on
  • Use a StringBuilder to build up the string
  • Declare the variable for the output string for the current line within the loop
  • Use if / else if to make it clear you're only expecting to go into one branch
  • Combine the two paths that both append the character as-is
  • Fix the condition for numbers (it's incorrect at the moment)
  • Use more whitespace for clarity
  • Specify a locale in toLower to avoid "the Turkey problem" with I

So:

String line;
while((line = file.readLine()) != null)
{
    StringBuilder builder = new StringBuilder(line.length());
    for (int i = 0; i < line.length(); i++) {
        char current = line.charAt(i);
        // Are you sure you want to trim 0?
        if ((current >= '1' && current <= '9') ||
            (current >= 'a' && current <= 'z')) {
            builder.append(current);
        } else if (current >= 'A' && current <= 'Z') {
            builder.append(Character.toLowerCase(current, Locale.US));
        }
    }
    System.out.println(builder);
}

people

See more on this question at Stackoverflow