0

I try to write a keyholder, and I want to write the passwords to a .dat file using ObjectOutputStream, and then read them using ObjectInputStream. This is my code for writing the objects:

public void toFile()
{    
    try
    {
        ObjectOutputStream oos = new ObjectOutputStream(new FileOutputStream("passwords.dat"));     
        for(int i = 0; i<this.nrOfPW; i++)
        {
            if(this.PWlist[i] instanceof longPW)
            {
                oos.writeObject((longPW)this.PWlist[i]);
            }
            else
            {
                oos.writeObject((PinPW)this.PWlist[i]);
            }   
        }
        oos.close();
    }
    catch(IOException e)
    {
        e.getStackTrace();
    }
}

This seems to work, but when I try to read the file again and put the objects in my PWlist array it says that PinPW isn't serializable, even though PinPW implements Serializable and it's imported. The base class of PinPW (Info) also implements Serializable and imports it. This is the code where I read the file:

public void fromFile() 
{
    try 
    {
        ObjectInputStream objIn =  new ObjectInputStream(new FileInputStream("passwords.dat"));
        while(objIn.readObject() != null)
        {
            if(this.nrOfPW == this.PWlist.length)
            {
                expand(10);
            }
            if(objIn.readObject() instanceof PinPW)
            {
                this.PWlist[this.nrOfPW] = (PinPW)objIn.readObject();
                this.nrOfPW++;
            }
            else
            {
                this.PWlist[this.nrOfPW] = (longPW)objIn.readObject();
                this.nrOfPW++;
            }
        }
        objIn.close();
    }
    catch(EOFException e)
    {
        e.getStackTrace();
    }
    catch(IOException ex)   
    {
        ex.printStackTrace();   
    }
    catch(ClassNotFoundException ex)
    {
        ex.printStackTrace();   
    }
}

The PWlist array is a Info array, and PinPW and longPW extends Info.

What do I do to fix this problem?

EscalatedQuickly
  • 400
  • 4
  • 22
  • Could you post the exception and stack trace that is generated - see my answer for a possible fix to this problem, and a *definite* fix to a bug. – Greg Kopff Jun 07 '12 at 21:42
  • 1
    [Off-topic] By convention, Java classes are named using UppercaseStartingCamelCase -- this means your `longPW` class should be `LongPW`. Similarly, variables are named using lowercaseStartingCamelCase, so your `PWList` variable should be called `pwList`. – Greg Kopff Jun 07 '12 at 21:43
  • You can simplify your method for creating the file. You don't need to check the type of the object nor cast them when calling `writeObject` since the parameter here is `Object`. You must though ensure that all passed objects implement interface `java.io.Serializable` as this is not enforced by the compiler (it is enforced by the serialization output stream). – Kevin Brock Jun 08 '12 at 01:26

3 Answers3

1

Let's fix the "first bug, first" ...

In this code:

while(objIn.readObject() != null)                         // reads object, tests then *discards* it
{
  ...

  if(objIn.readObject() instanceof PinPW)                 // reads object, tests then *discards* it
  {
    this.PWlist[this.nrOfPW] = (PinPW)objIn.readObject(); // conditionally read an object
    this.nrOfPW++;
  }
  else
  {
    this.PWlist[this.nrOfPW] = (longPW)objIn.readObject(); // conditionally read an object
    this.nrOfPW++;
  }
}

Each time around your loop iteration, you actually read 3 objects. The first time you read an object to check there was one in the stream, the next time you read one and determine it's type, then discard it. Then you read a third object and cast it based on what the type of the discarded object was.

In addition, as EJP correctly points out, the correct way to determine End of Stream for an ObjectInputStream is to catch the end of file exception.

You want to do this instead:

 try
 {
   while (true)
   {
     final Object o = objIn.readObject();            // read the object from the stream

     ...

     if (o instanceof PinPW)
     {
       this.PWlist[this.nrOfPW] = (PinPW) o;         // cast to correct type
       this.nrOfPW++;
     }
     else
     {
       this.PWlist[this.nrOfPW] = (longPW) o;        // cast to correct type
       this.nrOfPW++;
     }
   }
 }
 catch (EOFException e)
 {
   // end of stream reached ...
   // ... close the file descriptor etc ...
 }
Community
  • 1
  • 1
Greg Kopff
  • 15,945
  • 12
  • 55
  • 78
  • @Psyberion There is no reason to test for null at all, unless you are writing nulls when you serialize. The correct test for EOS on a serialized stream is to catch `EOFException`. – user207421 Jun 08 '12 at 00:10
0

You have a problem here.

while(objIn.readObject() != null)
    {
        if(this.nrOfPW == this.PWlist.length)
        {
            expand(10);
        }
        if(objIn.readObject() instanceof PinPW)
        {
            this.PWlist[this.nrOfPW] = (PinPW)objIn.readObject();
            this.nrOfPW++;
        }
        else
        {
            this.PWlist[this.nrOfPW] = (longPW)objIn.readObject();
            this.nrOfPW++;
        }
    }

You are reading an object many times. Try to save it and then work with it. if(objIn.readObject() instanceof PinPW) reads one, reads twice, this.PWlist[this.nrOfPW] = (PinPW)objIn.readObject(); reads three times, when it should be only once. PS: use Greg Kopff syntax inside a while and without the final keyword because you want to save more objects in it.

Claudiu C
  • 95
  • 1
  • 1
  • 11
0

I just wanted to point out the if-else block in the toFile() function is completely pointless. writeObject() takes an Object argument. It doesn't care what type of Object it is, as long as it's serializable.

While your fromFile() method is seriously flawed, it would not cause a NotSerializableException. I believe the exception you're referring to actually happened in toFile().

The cause is very simple: you didn't fully read and understand the documentation of ObjectOutputStream. To be specific, the Object and all its non-transient fields and all non-transient fields in its ancestor classes has to implement the Serializable. It must also have a public no-arg constructor.

billc.cn
  • 7,187
  • 3
  • 39
  • 79
  • *"It must have a public no-arg constructor"* - that is true for an `Externalizable` class, but not a `Serializable` class. – Greg Kopff Jun 07 '12 at 22:31
  • Well, you're right, this is not a strict requirement, but I always had those because there are complications with sub-classing if they're not there. Couldn't be bothered to remember all the rules on this, so I just unify it this way :) – billc.cn Jun 07 '12 at 22:34
  • You're thinking of [this](http://docs.oracle.com/javase/6/docs/api/java/io/Serializable.html): *"To allow subtypes of non-serializable classes to be serialized, the subtype may assume responsibility for saving and restoring the state of the supertype's public, protected, and (if accessible) package fields. The subtype may assume this responsibility only if the class it extends has an accessible no-arg constructor to initialize the class's state."* – Greg Kopff Jun 07 '12 at 22:42