0

I am trying to delete a text line from a file. So far I have this but it is giving me some problems. It works if there is nothing after the initial text on the line. But if in the text file I have anything after Bart ie Bart Jones for example, it will not delete the line, it will just leave it alone. Please help.

public void removeLineFromFile(String file, String lineToRemove) {

    try {

        File inFile = new File(file);

        if (!inFile.isFile()) {
            System.out.println("Parameter is not an existing file");
            return;
        }

        //Construct the new file that will later be renamed to the original filename.
        File tempFile = new File(inFile.getAbsolutePath() + ".tmp");

        BufferedReader br = new BufferedReader(new FileReader(file));
        PrintWriter pw = new PrintWriter(new FileWriter(tempFile));

        String line = null;

        //Read from the original file and write to the new
        //unless content matches data to be removed.
        while ((line = br.readLine()) != null) {

            if (!line.trim().equals(lineToRemove)) {

                pw.println(line);
                pw.flush();
            }
        }
        pw.close();
        br.close();

        //Delete the original file
        if (!inFile.delete()) {
            System.out.println("Could not delete file");
            return;
        }

        //Rename the new file to the filename the original file had.
        if (!tempFile.renameTo(inFile))
            System.out.println("Could not rename file");

    }
    catch (FileNotFoundException ex) {
        ex.printStackTrace();
    }
    catch (IOException ex) {
        ex.printStackTrace();
    }
}

public static void main(String[] args) {
    FileUtil util = new FileUtil();
    util.removeLineFromFile("booklist.txt", "bart");
}

} `

4 Answers4

1

Instead of .equals(lineToRemove) use .contains(lineToRemove)

pedrop
  • 134
  • 1
  • 7
1

Simply change

if (!line.trim().equals(lineToRemove))

with

if (!line.indexOf(lineToRemove) > -1)
  • There is no need to trim as you only want to know if the string is in the line.
  • indexOf generate less bytecode then contains as contains itself call indexOf with other trims.
  • You might want to compare using toLowerCase if you don't matter of the case.

See Is String.Contains() faster than String.IndexOf()? for more info on comparing with indexOf vs contains.

Community
  • 1
  • 1
Jean-François Savard
  • 20,626
  • 7
  • 49
  • 76
  • 1
    Readability trumps efficiency except in the most performance sensitive cases. I would use String#contains since it makes the intention clear as opposed to indexOf. – Ali Cheaito Mar 11 '15 at 15:27
  • 1
    It is shorter and more readable than `indexOf(lineToRemove) > -1`. That's premature optimization, even micro-optimization. – m0skit0 Mar 11 '15 at 15:27
  • @Jean-FrançoisSavard You linked to a C# question btw. – m0skit0 Mar 11 '15 at 15:28
  • It's your opinion. This practice is always the base of debates... To me it simply seems more logical to call the method directly instead of calling a method that will call another one. comparing with -1 is not less redable imo. As for the link, yes I realized, I deleted the link. However, you can search this on google for java if you want. – Jean-François Savard Mar 11 '15 at 15:29
  • Let's just see which suggestion OP will prefer. – Jean-François Savard Mar 11 '15 at 15:30
  • This question is more relevant to this discussion than the one you posted: http://stackoverflow.com/questions/10714376/which-string-method-contains-or-indexof-1 – m0skit0 Mar 11 '15 at 15:31
  • @m0skit0 I also saw this link, that's why I say it's simply a matter of opinion/discussion as you said. – Jean-François Savard Mar 11 '15 at 15:38
1

You need to use not equals but contains in this line:

if (!line.trim().equals(lineToRemove)) {

Like this:

if (!line.contains(lineToRemove)) {
Eirenliel
  • 345
  • 4
  • 14
1

It has to do with the way you're searching for the string in the file. The goal is to delete a line "containing" a string, not "equals to" a string. Changing the if statement in the loop should do the trick.

 if (!line.trim().toLowerCase().contains(lineToRemove.toLowerCase())) {

            pw.println(line);
            pw.flush();
        }

Notice that I also added a call to toLowerCase() in case the search string and line content where in different cases, you'd probably want to delete them as well. If that's not the case, you can safely remove those calls.

Ali Cheaito
  • 3,746
  • 3
  • 25
  • 30