I'm trying to take data from an ArrayList of a class and write it into a text file. It creates the temporary file, but doesn't do anything with it. It prints what I'm trying to put in the file and doesn't delete the temporary file. What am I doing wrong?
try
{
File temp = new File("temp.txt");
File file = new File(prop.getProperty("path"));
BufferedWriter writer = new BufferedWriter(new FileWriter(temp));
ArrayList<Contact> contacts = gestor.checkData();
if(temp.createNewFile())
{
for(int i = 0; i < contacts.size(); i++)
{
writer.write(contacts.get(i).getName());
writer.write(contacts.get(i).getLastNames());
DateFormat df = new SimpleDateFormat("yyyy-MM-dd");
String date = df.format(contacts.get(i).getBirthday());
writer.write(date);
writer.write(contacts.get(i).getEmail());
}
writer.close();
file.delete();
temp.renameTo(file);
}
}
catch (IOException e)
{
e.printStackTrace();
}
The code of checkData() just returns the ArrayList<Contact> contactList.
Many problems with your code:
Something like a new FileWriter is a resource. Resources MUST be closed. As a consequence, you should never make resources (you usually know it when you make a resource: you either call new X where X is clearly representative of a resource, or you call something like socket.getInputStream or Files.newBufferedReader... unless you do it 'right'. There are only two ways to do it right:
try (FileWriter w = new FileWriter(...)) {
// use it here
}
or, if you need the resource to be a field of your class, then the only safe way is to turn your class into a resource: make it implements AutoClosable, make a close method, and now the burden of using that try syntax is on whomever uses your class. There is no way to safely do this stuff without try-with-resources*.
files are bytes. not characters. In addition, the filesystem has no clue what the 'encoding' is. How do you turn ☃, or for that matter, é into bytes? The answer depends on charset encoding. The problem with all methods that turn bytes into characters or vice versa that are NOT in the java.nio.file package is that they use 'platform default encoding'. That's a funny way of saying 'the worst idea ever', as that will 'work' on your machine and will pass all tests, and then fail in production, at the worst possible moment. The solution is to never rely on platform default, ever. If you really really intend to use that, make it explicit. Unfortunately, FileWriter, until java11, has no way of specifying charset encoding, which makes it an utterly useless class you cannot actually ever use without writing buggy code. So don't. I suggest you switch to the new file API, which defaults to UTF-8 (quite a sane default), and can do a lot of complicated things in one-liners.
You just .write all this data. write does exactly what it says, and writes precisely the string. It does not print anything more. Specifically, it does not print any newlines. Surely you did not intend to just write, say, JoeSteel1990-10-01 to the file, all in one jumbled mess like that? write \n to avoid this, or preconstruct the entire string just as you want it (with newlines), and then write that.
Your code will silently do nothing if the 'temp' file already exists. It sounds like the design is that this is a weird situation (as the temp file is 'renamed' right after). As a general rule of thumb, 'silently do nothing' is entirely the wrong instinct to have when you run into scenarios you know are unlikely or seemingly impossible. The right instinct is the exact opposite: Fail as spectacularly as you can. (The best option, of course, is to think about what the weird scenario means and handle it properly, but that's not always possible). So, instead of that, try:
if (!temp.creatNewFile()) {
throw new RuntimeException("That is weird - tempfile already exists: " + temp);
}
That's the right mindset: If weird things happen, blow up, as fast as you can, with enough detail so you know what's going wrong. (The most likely 'correct' way to handle this is to just delete the temp file. The whole point of that temp file is that if it's still there when you enter this code, that the previous attempt failed halfway through, so just delete the product of the failed operation, it isn't useful).
Whilst common in examples and even IDE templates, you're not doing it right. You should NEVER handle an exception by printing something and carrying on. Think about it: If something went wrong, continuing with the code execution is either going to cause nasty problems (as you surely did not consider that one of the steps in your program failed when you write your code), or is going to cause another error. And if all errors are handled like this, one thing goes wrong and you get 85 error traces in your logs. That's not useful. If a problem occurs and you do not know how to handle it, do NOT continue running. The only sensible 'I do not know how to handle this' exception handling is:
catch (IOException e) {
throw new RuntimeException("unhandled", e);
}
sometimes a better exception than RuntimeException is possible (such as UncheckedIOException or ServletException, it depends on the situation). Also, sometimes the right answer is to just throws the exception onwards. Remember, public static void main can (and usually should!) be declared as throws Exception.
try {
Path temp = Paths.get("temp.txt");
Path file = Paths.get(prop.getProperty("path"));
ArrayList<Contact> contacts = gestor.checkData();
try (BufferedWriter writer = Files.newBufferedWriter(temp)) {
for (Contact contact : contacts) {
writer.write(contact.getName());
writer.write("\n");
writer.write(contact.getLastNames());
writer.write("\n");
DateFormat df = new SimpleDateFormat("yyyy-MM-dd");
String date = df.format(contact.getBirthday());
writer.write(date);
writer.write("\n");
writer.write(contact.getEmail());
writer.write("\n");
}
}
Files.delete(file);
Files.move(temp, file);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
The above will probably fail, but it will fail telling you exactly why it failed. For example, it might tell you that the directory you are trying to write to does not exist, or you have no write access. Whereas your code will then silently just do nothing.
*) For you java pros out there, sure, you can handroll your own try/finally loops. Let me know how many programmers new to java ever managed to dodge every mine in the minefield when you do that. Until you are fairly wintered, this rule of thumb becomes effectively a rule of law: You can't do it safely without t-w-r. Let's keep it simple.