search

pilgr / Paper

Paper is a fast NoSQL-like storage for Java/Kotlin objects on Android with automatic schema migration support.
Apache License 2.0
2.35k stars 234 forks source link

Fixed stream closing #98

Closed fanticqq closed 7 years ago

pilgr commented 7 years ago

To be honest I don't think those changes are needed.

  1. kryoOutput.flush() and kryoOutput.close() has to be called inside try{} to be handled in catch(KryoException).
  2. The backupFile.delete() must be called only if everything is OK. In your PR it's called before kryoOutput.flush().
  3. Closing underlying fileStream is not required. Calling for kryoOutput.close() closes underlying OutputStream as well.
  4. Having KryoException or IOException at this point means something terrible happened and app can't proceed normally, so we throw custom PaperDbException in order to provide more information about the problem. This means an app will crash at this moment and any of the unclosed streams doesn't matter.
pilgr commented 7 years ago

But anyway many thanks for the contribution! In case if you find any other problem it could be better to create an issue first so we can discuss if changes needed or not. This may save your time 💸

pilgr commented 7 years ago

btw I suppose you use Paper in one of your apps? If you can expose this info publicly – please add your app into https://github.com/pilgr/Paper/issues/92 Thanks!