I've made the following Java-class just for getting some practice with the creation and reading of files.
package modul;
import static java.lang.System.*;
import java.io.*;
public class Sandbox {
public static void main (String[] args) {
// I found out that the project directory is the
// working directory by default.
// Is there a way to change the working directory in Java?
// So that all instructions become relative to that directory.
String currentDir = System.getProperty("user.dir") + "/src/modul/";
String fileName = "lorem.txt";
try {
PrintWriter writer = new PrintWriter(new BufferedWriter(new FileWriter(currentDir + fileName)));
// Write a few lines of dummy-text ...
writer.println("The very first line of text.");
writer.println("----------------------------");
writer.println("Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aenean commodo ligula eget dolor.");
writer.println("Donec quam felis, ultricies nec, pellentesque eu, pretium quis, sem. Nulla consequat massa quis enim.");
writer.println("In enim justo, rhoncus ut, imperdiet a, venenatis vitae, justo.");
// Is it enough just closing the PrintWriter?
// What's about the BufferedReader and the FileReader?
// Are they closed automatically?
writer.close();
// Now open the created file. Read and print it's
// content to stdout.
BufferedReader reader = new BufferedReader(new FileReader(currentDir + fileName));
String line;
int lineNumber = 0;
while ((line = reader.readLine()) != null) {
out.println(++lineNumber + ": " + line);
}
// Is it fair enough just closing the Reader?
reader.close();
} catch (FileNotFoundException e) {
e.printStackTrace();
} catch (IOException e) {
e.printStackTrace();
} catch (Exception e) {
e.printStackTrace();
}
}
}
Although it won't serve a real use-case it's important for me to know if everything is done in a good way. Or does I have to improve the code?
Moreover: Are there ways to accomplish writing-, reading-operations more efficient? Respectively in a simpler way?
A few more questions I've still got about Java IO-operations I've added as comments within the code.
All comments, hints and recommendations of more experienced Java-devs much appreciated.
-
2\$\begingroup\$ It is considered standard on Code Review to leave the question open (i.e., no accepted answers) for at least a day so that new answerers have motivation to answer, existing answers are be improved, etc. \$\endgroup\$Tamoghna Chowdhury– Tamoghna Chowdhury2017年05月05日 10:50:24 +00:00Commented May 5, 2017 at 10:50
-
\$\begingroup\$ @TamoghnaChowdhury Sorry. Will keep that in mind for the future. And thanks for the hints. \$\endgroup\$michael.zech– michael.zech2017年05月06日 02:54:30 +00:00Commented May 6, 2017 at 2:54
4 Answers 4
Nice first implementation.
Autoclosable / try-with-resources
Please consider the AutoClosable feature from Java (削除) 8 (削除ここまで) 7 to use in a try-with-resources block. See here for more detail.
Split try blocks to be as small as possible
Try (ha :) to isolate as much code as possible in your try blocks. You can make better error handling that way.
Reuse classes
Always search the Java API for nice helper classes, in this case there is a LineNumberReader that will do what you want to do
Changing working directory
I really hate it when programs do this. You will (on exit) end up in a different locatation, unless you put effort in switching back, which makes you program more difficult.
Best thing to do it to establish a 'base' directory, and use relative paths from there. Just like you did.
Proposed solution
try (PrintWriter writer = new PrintWriter(new BufferedWriter(new FileWriter(currentDir + fileName))))
{
// Write a few lines of dummy-text ...
writer.println("The very first line of text.");
writer.println("----------------------------");
writer.println("Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aenean commodo ligula eget dolor.");
writer.println("Donec quam felis, ultricies nec, pellentesque eu, pretium quis, sem. Nulla consequat massa quis enim.");
writer.println("In enim justo, rhoncus ut, imperdiet a, venenatis vitae, justo.");
} catch (FileNotFoundException e) {
e.printStackTrace();
} catch (IOException e) {
e.printStackTrace();
} catch (Exception e) {
e.printStackTrace();
}
try(LineNumberReader lineLeader = new LineNumberReader( new BufferedReader(new FileReader(currentDir + fileName))))
{
// Now open the created file. Read and print it's
// content to stdout.in
String line;
while ((line = lineLeader.readLine()) != null) {
out.println(lineLeader.getLineNumber() + ": " + line);
}
}
catch (FileNotFoundException e) {
e.printStackTrace();
} catch (IOException e) {
e.printStackTrace();
} catch (Exception e) {
e.printStackTrace();
}
-
\$\begingroup\$ The only thing I miss in your answer is the piping of Exceptions. \$\endgroup\$slowy– slowy2017年05月05日 09:12:18 +00:00Commented May 5, 2017 at 9:12
-
\$\begingroup\$ You cannot 'pipe' (multi-catch) the exceptions, because the
Exception
will eat them all :) \$\endgroup\$Rob Audenaerde– Rob Audenaerde2017年05月05日 09:23:52 +00:00Commented May 5, 2017 at 9:23 -
3\$\begingroup\$ Ooooh, right, my bad (low coffee level). Then I'd suggest do get rid of FileNotFound- and IOException, with the reason, that the handling of those is the same as in Exception anyway. \$\endgroup\$slowy– slowy2017年05月05日 09:33:20 +00:00Commented May 5, 2017 at 9:33
-
\$\begingroup\$ Thanks for the bunch of awesome answers. Sadly I can accept only one answer. But they all were useful to me. Definitely! \$\endgroup\$michael.zech– michael.zech2017年05月05日 10:05:23 +00:00Commented May 5, 2017 at 10:05
-
2\$\begingroup\$ I do not know java that much, but why would you (as a user) end in a different directory? Working dir is a property of a process, so the java process will not change the working directory of the user's shell. \$\endgroup\$Edheldil– Edheldil2017年05月05日 11:54:32 +00:00Commented May 5, 2017 at 11:54
PrintWriter writer = new PrintWriter(new BufferedWriter(new FileWriter(currentDir + fileName))); ... BufferedReader reader = new BufferedReader(new FileReader(currentDir + fileName));
Never ever ever open a file for reading or writing characters without explicitly specifying the encoding. You'll think it's all working fine and then someone will run your program on Mac OS X and send you a file encoded in MacRoman because that's the default on their platform.
Yes, I learnt that one the hard way.
Unfortunately the Java standard library tries to make things "easy" by hiding access to the encoding and ends up making it hard for anyone who wants to write portable code (supposedly Java's strong point). You can't use FileWriter
, because none of its constructors have an encoding parameter. You have to create a FileOutputStream
and wrap it in an OutputStreamWriter
. FileReader
is similarly useless.
-
\$\begingroup\$ +1 for the encoding. Very important. Cost me loads of time in the past as well. In this example the default encoding is used twice, so it won't be an issue. \$\endgroup\$Rob Audenaerde– Rob Audenaerde2017年05月05日 08:55:05 +00:00Commented May 5, 2017 at 8:55
-
\$\begingroup\$ btw. Apache commons provides the very useful
FileWriterWithEncoding
\$\endgroup\$Rob Audenaerde– Rob Audenaerde2017年05月05日 09:00:34 +00:00Commented May 5, 2017 at 9:00 -
\$\begingroup\$ +1, encoding can be a PITA when you go multiplatform - but there's one point you got incorrect - the constructors for the IO stream classes have a parameter
String csn
, by which you can specify the charset to use. See my answer for details. BTW, nice Newton in your DP! \$\endgroup\$Tamoghna Chowdhury– Tamoghna Chowdhury2017年05月05日 09:02:09 +00:00Commented May 5, 2017 at 9:02 -
\$\begingroup\$ @RobAu that's not really necessary. See my answer. \$\endgroup\$Tamoghna Chowdhury– Tamoghna Chowdhury2017年05月05日 09:02:38 +00:00Commented May 5, 2017 at 9:02
-
\$\begingroup\$ @TamoghnaChowdhury, thanks for the correction. The source code for generating the profile pic is at codegolf.stackexchange.com/a/2543/194 \$\endgroup\$Peter Taylor– Peter Taylor2017年05月05日 09:55:41 +00:00Commented May 5, 2017 at 9:55
Funny how nowbody seems to know the Files
class...
I agree with the previous answers, that try-with-resources is a good thing to do, but to perform the task in a real simple way, use Files
:
// In reality, you practically always have your text in some kind of data structure:
String[] text = {
"The very first line of text.\n", // Note, that I added the newlines manually
"----------------------------\n", // as Files.write does not do this for you
"Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aenean commodo ligula eget dolor.\n",
"Donec quam felis, ultricies nec, pellentesque eu, pretium quis, sem. Nulla consequat massa quis enim.\n",
"In enim justo, rhoncus ut, imperdiet a, venenatis vitae, justo.\n" };
// Replaces the complete writing part:
Files.write(Paths.get("filename"), Arrays.asList(text));
// Replaces the complete reading part:
Files.readAllLines(Paths.get("filename")).forEach(System.out::println);
(Please have a look at the API docs, there are also variants to explicitly set the character set https://docs.oracle.com/javase/8/docs/api/java/nio/file/Files.html)
-
\$\begingroup\$ Great suggestion! Btw. you could use a nice lambda+map to append the
'\n'
after each line :) \$\endgroup\$Rob Audenaerde– Rob Audenaerde2017年05月05日 09:34:22 +00:00Commented May 5, 2017 at 9:34 -
\$\begingroup\$ I'm working on mentioning it, but +1 for getting there earlier. \$\endgroup\$Tamoghna Chowdhury– Tamoghna Chowdhury2017年05月05日 10:17:37 +00:00Commented May 5, 2017 at 10:17
-
\$\begingroup\$ There is a write overload that accepts encoding \$\endgroup\$Basilevs– Basilevs2017年05月05日 11:58:37 +00:00Commented May 5, 2017 at 11:58
I'll get started on the major things first - the I/O. Stylistic ideas will come later.
Try-With-Resources
If you're using Java 7+, use the try-with-resources statement. The syntax is similar to C#'s
using
statement:try (PrintWriter writer = new PrintWriter(new BufferedWriter(new FileWriter(currentDir + fileName)))) { // Do something using `writer`, it'll be closed automatically when it goes out of scope of the try-with-resources } catch (/* Whatever exceptions you need to handle*/){ // Do something to handle the exception } // Look ma! No `finally`!
Note that
PrintWriter
's methods don't throw any exceptions - you need to callcheckError()
onwriter
to find out if any errors occurred. Internal calls may produce exceptions which may propagate up toPrintwriter
, but that's not the general case. (Check the JavaDoc here.)The same for reading the file. Here my stylistic suggestion (2) should come in useful.
Alternative Constructors and Encoding
PrintWriter
has a constructorPrintWriter(String fileName, String csn)
, which does exactly what you do manually. You could just provide it the file path and it'll work. No need to manually createBufferedWriter
andFileWriter
objects.What is that
String csn
in the second parameter? There's an overload of this constructor without thecsn
parameter - ideally, as @Peter Taylor said, you shouldn't use it. Why? "CSN" stands for "CharSet Name", or the output encoding. You'll generally want to pass inStandardCharsets.UTF_8.displayName()
, which should be"UTF-8"
(case insensitive) forcsn
. You can findStandardCharsets
by importingjava.nio.charset.StandardCharsets
. Encoding can be a problem when you go multiplatform.Note that
BufferedReader
orFileReader
do not have such acsn
parameter in any of their constructors. What you would want to do here if have Java 7+ is to use the classjava.nio.file.Files
'newBufferedReader(Path path, Charset charset)
method to create theBufferedReader
with the specified charset.You might call it like this:
// Ensure that the following classes have been imported properly // The IDE should help BufferedReader reader = Files.newBufferedReader(FileSystems.getDefault().getPath(currentdir, fileName), StandardCharsets.UTF_8);
or, a slightly more succinct version:
BufferedReader reader = Files.newBufferedReader(Paths.get(currentdir + fileName), StandardCharsets.UTF_8);
The relevant JavaDocs are here:
a. Files
b. FileSystems
c. FileSystem
e. Paths
close()
infinally
If you aren't using Java 7 and above, the recommendation is to put all
close()
calls inside thefinally
statement after all thecatch
es (the "Ye Olde" way). For this, your IO streams should be declared outside the try-catch-finally block.It could look like this:
PrintWriter writer; try { // Using my suggestion from (2) writer = new PrintWriter(currentDir + fileName, "UTF-8"); } catch(/* Whatever exceptions you need to handle*/) { // Do something to handle the exception } finally { writer.close(); }
Stylistic suggestions:
It is fair enough to close only the outermost stream - the close request will be propagated by the stream to any enclosed ones.
You should make reading and writing as 2 different methods,
readFile
andwriteFile
. You may even want to parametrize them to take into account different files (file paths) and contents to be written.Your variable names are good and descriptive, nothing to fault there.
Note that Java convention states that project names should be unique and involve a package structure involving the reversed domain name associated with the publisher of the software, i.e., your package structure should be something like
com.mizech.modul
. This ensures uniqueness in qualified names to a great extent.Another place where your naming scheme falters is in the naming of the exception objects. If we go with the scheme as object name = lowercase initials (first letter of each camelCased word) of type name, it should be
fnfe
forFileNotFoundException
andioe
forIOException
- but see point 9 below.I don't recommend the use of
import static java.lang.System.*
, as it may lead to namespace pollution, especially in an I/O oriented app like yours.I would really suggest you take a look at
LineNumberReader
- that very C-ishwhile
loop is not really considered good Java.LineNumberReader
will automatically keep track of the line number for you - otherwise, it's just aBufferedReader. You can get the current line number with it's
getLineNumber()` method, however, note that the line number returned is 0-based. You might have to add 1 to the result to make it work like yours currently does.You really should look into
java.nio.file.Files
(JavaDoc linked above). It has utility methods for a lot of stuff - writing and reading files, file and directory management (copying, moving, deleting), etc. An exposition of the Java 8-enhanced properties of this class has already been provided by @mtj, and this answer's already pretty long, so I won't get into it here. Suffice it to say that most of what you've done is redundant and Java does indeed have a simpler and probably more efficient way of dealing with files -java.nio
and subpackages therein.Please don't change the current directory for the reasons specified by @RobAu - it's very disorienting if your current working directory (CWD) gets changed if your app has been run from the terminal (using, say,
java Sandbox
, whereSandbox.class
is in the CWD of the shell). If launched separately from the IDE using a terminal or console, the application's CWD will be the same as that of the launching console.If you absolutely have to do it, though, you can try
System.setProperty("user.dir", "/foo/bar/");
, where/foo/bar
will be whatever directory you need to be the current working directory.If you just want to use a common relative root for all paths, you can rely on
java.nio
'sPath
objects'resolve
method. They'll kind of do what you want - resolve the provided relative path string with the specifiedPath
as the root.If you're handling all
Exception
s the same way, then you might was well keep only the most generalcatch
,catch (Exception e)
(thanks @slowy!). Don't Repeat Yourself!