Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link
  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 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

  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

  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

adds some clarifications
Source Link
palacsint
  • 30.3k
  • 9
  • 82
  • 157
  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

  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 StringBuilder although FileUtils is better.)

  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

  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

Source Link
palacsint
  • 30.3k
  • 9
  • 82
  • 157
  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 StringBuilder although FileUtils is better.)

  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

lang-java

AltStyle によって変換されたページ (->オリジナル) /