list -> file -> list -> error

[Pobunjenik]

Hi!
Straight to the problem:
I'm building this server application which will monitor SQL databases. I want it to remember any number of connections so I've made a few methods which I thought would do the trick, except they don't.
The program should add any new SQL connection to a List<MyConnection>, then serialize it to a file "connections.txt", and finally to a JComboBox.

PROBLEM:
When I add the first connection, it works properly since the file needs to be generated.
Then I proceed to add a second one, and for that I found that I have to delete the file to generate a new one in it's place.
In theory, that one should contain both connections, but it only returns the first one to the JCB and it catches this error:
java.io.WriteAbortedException: writing aborted; java.io.NotSerializableException: com.mysql.jdbc.SingleByteCharsetConverter

CODE:
This method saves the list to a file:

public void saveList(String name) {
        File file = new File(name);
        if (file.exists()) {
            file.delete();
            System.out.println("I deleted " + name);
        }
        try {
            ObjectOutputStream oos;
            try (FileOutputStream fos = new FileOutputStream(name)) {
                oos = new ObjectOutputStream(fos);
                Iterator it = Connectionslist.iterator();
                while (it.hasNext()) {
                    oos.writeObject((MyConnection) it.next());
                    System.out.println("Serialized: " + (MyConnection) it.next());
                }
                fos.flush();
                fos.close();
            }
            oos.flush();
            oos.close();
        } catch (Exception e) {
            System.out.println("Problem serializing: " + e);
        }
    }

This method reads from the same file:

public void loadList(String name) throws FileNotFoundException, IOException, ClassNotFoundException {
        Connectionslist.clear();
        ObjectInputStream ois;
        try (FileInputStream fis = new FileInputStream(name)) {
            ois = new ObjectInputStream(fis);
            Connectionslist.add((MyConnection) ois.readObject());
            System.out.println("Deserialized: " + (MyConnection) ois.readObject());
            fis.close();
        }

        ois.close();
    }

And this one takes objects from the list and puts them in a combobox:

public JComboBox list2combobox(List list, JComboBox jcb) {
        jcb.removeAllItems();
        for (Object item : list) {
            jcb.addItem(item);
        }
        return jcb;
    }

[Pobunjenik]

This is the object I'm trying to serialize (plus get/sets methods which I didn't copy).

public class MyConnection implements Serializable{

    String port;
    String databaseName;
    String username;
    String password;
    String host;
    String nickname;
    String URL;
    private Connection thisConnection;

    public MyConnection (String host, String port, String databaseName, String username, String password, String nickname) throws SQLException {
        Connection newConnection = this.thisConnection;
        this.host = host;
        this.port = port;
        this.databaseName = databaseName;
        this.username = username;
        this.password = password;
        this.nickname = nickname;
        this.URL = "jdbc:mysql://" + this.host + ":" + this.port + "/" + this.databaseName;

    }

[stultuske]

Can you print out the actual stacktrace?

[JamesCherrill]

What do you intend with thisConnection and newConnection? They don't seem to make a lot of sense.

Anyway, you can only serialise a class that implements Serialisable (so check its API doc). Some classes (eg ones that are intrinsically tied to some operating system resource and use its state) are not able to be written to file and then restored by just reading them back in.

[Pobunjenik]

You're right James. Connection newConnection = this.thisConnection; is a leftover line from something I tried earlier.

The 'MyConnection' class does implement Serializable. It all works as intended for the first connection. The problem seems to occur when I try to save a second connection (or third).

Why does it correctly serialize/deserialize/ one connection, but fail to do so with multiple connections? Could the problem be my loadList() method since that one has no iteration and the saveList() one has?

[JamesCherrill]

Yes - that's definitely a problem.
Your load will have to use a loop to read all the instances.

Alternatively (easier) is ConnectionList is an ordinary Java Collection, you cam simply write that as a single object to the OOS and read it as a single object from the OIS. That will serialise the list, and all the objects in it, in a single call.

[Pobunjenik]

Hmmm, yes. I'll rework the code to use a Collection instead of a List. I'll do that today and then I'll report back!

(I can't do it right away as I'm in a class, doing Cryptology ATM.)

[JamesCherrill]

No need to change that! A List is a Collection.

(interface List extends Collection, All Known Implementing Classes:
AbstractList, AbstractSequentialList, ArrayList, AttributeList, CopyOnWriteArrayList, LinkedList, RoleList, RoleUnresolvedList, Stack, Vector)

I was just making the point that you can read/write the whole thing as a single object, as long as it's some kind of Java Collection, eg ArrayList, as opposed to a class of your own.

[Pobunjenik]

AH! Now I see!
You want me to serialize like so:

public void saveList(String name) {
        File file = new File(name);
        if (file.exists()) {
            file.delete();
            System.out.println("I deleted " + name);
        }
        try {
            ObjectOutputStream oos;
            try (FileOutputStream fos = new FileOutputStream(name)) {
                oos = new ObjectOutputStream(fos);
                oos.writeObject(Connectionlist); // serialized whole list
                                                 // no iteration over list
                fos.flush();
                fos.close();
            }
            oos.flush();
            oos.close();
        } catch (Exception e) {
            System.out.println("Problem serializing: " + e);
        }
    }

[JamesCherrill]

Yes, and similarly for reading it back in.

ps: It's easier for us to read your code if you stick to Java naming conventions - connectionList, not Connectionlist (looks like it's a class)

[~s.o.s~]

Two minor points:

1. You are ignoring the return value of the delete method; this is important if you are absolutely sure you want to have the file deleted since files opened up in other apps block deletion. Also it makes the logging statement incorrect in case the delete fails.
2. You have a redundant flush/close call. Calling close on the outermost stream will first flush the underlying stream and then close it.

[JamesCherrill]

... and
3. Since you have a try-with-resources, your FileOutputStream is guaranteed to be closed when you exit the block, either normally or abnormally - which is more than your explicit close calls will do!

[Pobunjenik]

Thank you everyone! I've followed your advice and the code now works as expected.

As per the code below, I serialized the whole collection at once, and deserialized it in the same manner (casting was required).

public void saveList(String name) {
        File file = new File(name);
        if (file.exists()) {
            file.delete();
            System.out.println("I deleted " + name);
        }
        try {
            ObjectOutputStream oos;
            try (FileOutputStream fos = new FileOutputStream(name)) {
                oos = new ObjectOutputStream(fos);
                oos.writeObject(connectionList);
            }
        } catch (Exception e) {
            System.out.println("Problem serializing: " + e);
        }
    }

    public void loadList(String name) throws FileNotFoundException, IOException, ClassNotFoundException {
        connectionList.clear();
        ObjectInputStream ois;
        try (FileInputStream fis = new FileInputStream(name)) {
            ois = new ObjectInputStream(fis);
            connectionList.addAll((Collection<? extends MyConnection>) ois.readObject());
        }
    }

[JamesCherrill]

Excellent! But why not just

connectionList = (Collection<? extends MyConnection>) ois.readObject();

[Pobunjenik]

I tried that, but connectionList is in interface Bridge (so I can call the same instance of it across the whole program). I guess that makes it final so using = redlines the code.
On the other hand collectionList.addAll(); works perfectly.

[JamesCherrill]

Yes, if you declare a variable in an interface it is assumed to be a constant (ie static final). Using it to hold a reference to an object with non-constant contents is valid, but somehow misses the point of it being final.

Creating a static final variable for your dynamic collection list seems like a dubious design decision - but not an actual error.

Anyway, as you correctly surmise, being static does indeed mean that you can't re-assign the variable, but you can remove and replace the contents of the object it refers to.

[Pobunjenik]

True all that, but declaring the list inside an interface allows me to easily call the same instance of the list from where ever need be!
That is important in this case since this application is to control/maintain/set up any number of SQL connections.

After this, I plan to build a chat service that allows you send/recieve encrypted messages via your own database (which will be controlled/monitored/set up by the server app).

[JamesCherrill]

Fair enough, but personally I would never use an interface to create a global shared variable. In this case maybe it would be cleaner to have a ConnectionManager class that encapsulates everything to do with the set of connections, eg tracks and catalogs them, saves/restores them to disk, makes them available to a GUI etc etc. If not, I suspect that as the application grows you will have more and more code that works with the connections, but that code will end up scattered all over the application.

[Pobunjenik]

If I start a new class called ConnectionManager, I'd have to do ConnectionManager cm = new ConnectionManager(); in everyother class that needs to work with a connection.
Apart from that, all my methods are in a class of mine called Engine. So I end up only having to do stuff like e.list2combobox(connectionList, servers_CB);

[JamesCherrill]

Definitely not! You just need one connection mamager for the whole application. You could create your manger instance when initialising the engine and share it from there- I guess everything has access to the engine instance?

Putting all your methods in an "engine" class may seem easy at first, but as the application grows it will turn into a monster. In the end it's always better to partition your code into modules (classes), each with one specific area of responsibity and a well-defined public interface. You then have one "controller" class that manages the whole thing.

I'm not suggesting you re-write this application now, but these are points to remember for the next one.

[Pobunjenik]

You're right. I'll group my methods based on what they affect (comming soon). :D
Right now, I have the following classes (after I did some tidying up by renaming stuff):

• GUIMain.java (main)
• GUIAddConnection.java
• GUIEditConnection.java
• SQLConnection.java
• Engine.java

And one interface: Bridge.java. The bridge contains the List<SQLConnection> connectionList.
Where/how would you implement a connection manager class? I fail to understand how I'd call the exact same instance of the list from there (ie. without using ConnectionManager cm = new ConnectionManager();).

[JamesCherrill]

Applications need a "master" or "controller" class that start up, manages and controls evrything else. It also acts a central point of reference that is accessible to the whole application (you often pass it as a parameter to the constructors of the various component classes). In this case it could have a getConnectionManager() method that anyone can call. Kinda like...

public static void main... {
   new Controller().start();
}

class Controller {

    private DefaultOptions defaults;
    private ConnectionManager connections;
    ...

   start() {
      defaults = new DefaultOptions();
      connections = new ConnectionMananger(defaults.getWorkingDirectory());
      Engine e = new Engine(this);
      ...

public ConnectionManager getConnectionManager() {
   return connections;
}   

[Pobunjenik]

Hmmm, yes. I understand the theory but I'm afraid it might be too late to implement that at this point.
It is certainly possible to complete this application of mine without using a Controller class.
I'll be sure to use this in the future!