7
\$\begingroup\$

I'm trying to write a simple class to represent a text file - which one would think should be quite easy. I don't intend to add all bells and whistles to fully represent a text file just enough to be able create/open read and write etc.

The more I look at it the more I think that it's a bad idea or rather that I'm approaching it the wrong way. The main problem that stands out is the exception handling which horrible. I've thrown runtime exceptions for situations that if feel there is no other way to handle them there must be a better way to do this.

The getText() method throws a FileNotFoundException which is ugly for the caller - before this I handled it in the method and created a new file and returned an empty string which is worse, I think.

Initially I read in the text only once on construction and stored the string in a field which suited my needs better (I don't expect the file to be changed and continually having to read it in seems a bit inefficient) but later I decided that this didn't really correctly model a TextFile.

How can I improve this class that should represent a text file?

  public class TextFile extends File{

    private static final long serialVersionUID = 1L;

    /**
     * -Create a new textfile, with the supplied text
     * -if the file already exists the text 
     *  it will be replaced with supplied text
     * 
     * @param pathname
     * @param text
     */
    public TextFile(String pathname, String text){
        super(pathname);
        if(!exists()) {
            createNewTextFile();
        }else {
            try {
                setText(text);
            } catch (FileNotFoundException e) {
                throw new RuntimeException(e);
            }
        }
    }


    /**
     * -Creates a new text file  
        by opening the file pointed to by the supplied pathname
     * -If the file doesn't  exist a new empty text file will be created
     * 
     * @param pathname
     * @throws IOException 
     */
    public TextFile(String pathname) {
        super(pathname);
        if(!exists()) {
            createNewTextFile();
        }
    }


    /*
     * return the text in the textFile
     */
    public String getText() throws FileNotFoundException {
        return read();
    }

    /**
     * Set the text in the text file
     * @param text
     * @throws FileNotFoundException 
     */
    public void setText(String text) throws FileNotFoundException {
        if(!exists()) {
            try {
                createNewFile();
            } catch (IOException e) {
                throw new RuntimeException(e);
            }   
        }
        write(text);
    }

    public void clearText() throws FileNotFoundException {
        setText("");
    }

    private String read() throws FileNotFoundException {
        FileInputStream fis = new FileInputStream(getPath());
        InputStreamReader isw = new InputStreamReader(fis);
        BufferedReader reader  = new BufferedReader(isw);
        StringBuffer fileText = new StringBuffer();

        String line;
        try {
            while ((line = reader.readLine()) != null) {
                  fileText.append(line+"\n"); 
            }
        } catch(IOException e) {
            throw new RuntimeException(e);
        } finally {
            try {
                if(fis != null) {
                    fis.close();
                }
                isw.close();
                reader.close(); 
            } catch (IOException e) {
                throw new RuntimeException(e);  
            }
        }
        return fileText.toString();
    }


    private void write(String text) throws FileNotFoundException{
        FileOutputStream fos = new FileOutputStream(getPath());
        OutputStreamWriter osw = new OutputStreamWriter(fos);
        BufferedWriter writer  = new BufferedWriter(osw);

        try {
            writer.write(text);
        } catch (IOException e) {
            throw new RuntimeException(e);
        }finally {
            try {
                writer.flush();
                writer.close();
                if(fos != null) {
                    fos.close();
                }
                osw.close();
            } catch (IOException e) {
                throw new RuntimeException(e); 
            }
        }
    }

    private void createNewTextFile() {
        try {
            createNewFile();
        } catch (IOException e) {
            throw new RuntimeException(e);
        }

    }


    /**
     *  Combine the text content from 
     *  a list of files into a single string
     *  
     * @param textFiles
     * @return
     * @throws FileNotFoundException 
     */
    public static String textFiles2String(List<TextFile> textFiles) 
                                     throws   FileNotFoundException{
        StringBuilder combined = new StringBuilder();   
        for(TextFile textFile : textFiles) {
            combined.append(textFile.getText());    
        }
        return combined.toString();
    }

}

Another version (still ugly but it makes more sense for my needs)

  • getText and setText only work with an in memory copy of the text...
  • Added commitChangesToFile method to update the file on the disk
  • Added reReadFile method to update the in memory copy

public class TextFile extends File{

    private static final long serialVersionUID = 1L;
    private String text;

    /**
     * -Create a new textfile, with the supplied text
     * -if the file already exists the text 
     *  it will be replaced with supplied text
     * 
     * @param pathname
     * @param text
     * @throws 
     */
    public TextFile(String pathname, String text) {
        super(pathname);
        this.text = text;

        if(!exists()) {
            createNewTextFile();
        }else {     
            try{
                write(text);
            } catch (FileNotFoundException e) {
                throw new RuntimeException();   
            }
        }
    }


    /**
     * -Creates a new text file  by opening the file pointed to by the supplied pathname
     * -If the file doesn't  exist a new empty text file will be created
     * 
     * @param pathname
     * @throws IOException 
     */
    public TextFile(String pathname) {
        super(pathname);
        if(!exists()) {
            createNewTextFile();
            text = "";
        }else{
            try{
                text = read();
            } catch (FileNotFoundException e) {
                throw new RuntimeException();   
            }
        }
    }


    /*
     * return the text in the textFile
     */
    public String getText() {
        return text;
    }

    /**
     */
    public void setText(String text){
        this.text = text;
    }

    public void clearText() {
        setText("");
    }

    public void commitChangesToFile() throws FileNotFoundException {
        write(text);
    }

    public void reReadFile() throws FileNotFoundException {
        text = read();
    }

    /**
     *  Combine the text content from 
     *  a list of files into a single string
     *  
     * @param textFiles
     * @return
     * @throws FileNotFoundException 
     */
    public static String textFiles2String(List<TextFile> textFiles) throws FileNotFoundException{
        StringBuilder combined = new StringBuilder();   
        for(TextFile textFile : textFiles) {
            combined.append(textFile.getText());    
        }
        return combined.toString();
    }

    private String read() throws FileNotFoundException {
        FileInputStream fis = new FileInputStream(getPath());
        InputStreamReader isw = new InputStreamReader(fis);
        BufferedReader reader  = new BufferedReader(isw);
        StringBuffer fileText = new StringBuffer();

        String line;
        try {
            while ((line = reader.readLine()) != null) {
                  fileText.append(line+"\n"); 
            }
        } catch(IOException e) {
            throw new RuntimeException(e);
        } finally {
            try {
                if(fis != null) {
                    fis.close();
                }
                isw.close();
                reader.close(); 
            } catch (IOException e) {
                throw new RuntimeException(e);  
            }
        }
        return fileText.toString();
    }


    private void write(String text) throws FileNotFoundException{
        FileOutputStream fos = new FileOutputStream(getPath());
        OutputStreamWriter osw = new OutputStreamWriter(fos);
        BufferedWriter writer  = new BufferedWriter(osw);

        try {
            writer.write(text);
        } catch (IOException e) {
            throw new RuntimeException(e);
        }finally {
            try {
                writer.flush();
                writer.close();
                if(fos != null) {
                    fos.close();
                }
                osw.close();
            } catch (IOException e) {
                throw new RuntimeException(e); 
            }
        }
    }

    private void createNewTextFile() {
        try {
            createNewFile();
        } catch (IOException e) {
            throw new RuntimeException(e);
        }

    }   
}
\$\endgroup\$
7
\$\begingroup\$
  1. I think you shouldn't use inheritance here. The java.io.File class is an abstract representation of file and directory pathnames (from the javadoc) and it has methods that a TextFile should not have: list, listFiles, mkdir, etc. Effective Java, 2nd Edition, Item 16: Favor composition over inheritance is a good source for this topic.

  2. Instead of the read and write methods I'd use FileUtils.readFileToString and writeStringToFile or at least check (or copy) their source. "... take advantage of the knowledge of the experts who wrote it and the experience of those who used it before you." (From Effective Java, 2nd Edition, Item 47: Know and use the libraries)

  3. You should specify the charset when you create the OutputStreamWriter. The default could vary from system to system and you may loose non-ASCII characters.

  4. Notice the difference between StringBuffer and StringBuilder: StringBuilder and StringBuffer in Java on Stack Overflow (The read method could use non-thread safe StringBuilder although FileUtils is better. You use the StringBuilders as local variables inside a method and currently they can't be accessed by other threads so it's enough to use non-thread-safe version.)

  5. The code currently throws RuntimeExceptions on IO errors and RuntimeExceptions usually stops the whole application. A well-written application or library should handle disks without empty space and other IO errors, so I think throwing checked IOExceptions would be fine here. Further reading: Effective Java, 2nd Edition, Item 58: Use checked exceptions for recoverable conditions and runtime exceptions for programming errors

\$\endgroup\$
  • 1
    \$\begingroup\$ Great answer, thanks again! I will have to check out Effective Java.. [1] Initally I didnt extend File - but then I thought hey why not i get all this stuff that I need.. I carelessly didnt think much more about it! [2] I have used the commons before, didn't spot that util though.. very handy and much better written then my read an write. [3] Had charsets in back of my mind, it was one of those things I ment to check out.. thanks for spotting this [4] this was a case of being better safe than sorry but... I dont think Ill have multiple threads operating on.. [5] Good point! \$\endgroup\$ – volting Sep 24 '12 at 21:44
  • \$\begingroup\$ "You use the StringBuilders as local variables inside a method and currently they can't be accessed by other threads so it's enough to use non-thread-safe version."- Good point - I think I need another cup of coffee!! :-) \$\endgroup\$ – volting Sep 24 '12 at 22:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.