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.34k stars 234 forks source link

mContext is useless when location is explicit in getBook() #200

Open Mino260806 opened 2 years ago

Mino260806 commented 2 years ago

the current Paper.init(Context) forces you to provide a context. However there's a case where mContext in Paper is not used at all: when location is explicit in getBook().

private static Book getBook(String location, String name) {
        if (mContext == null) {
            throw new PaperDbException("Paper.init is not called");
        }
        String key = (location == null ? "" : location) + name;
        synchronized (mBookMap) {
            Book book = mBookMap.get(key);
            if (book == null) {
                if (location == null) {
                    book = new Book(mContext, name, mCustomSerializers);
                } else {
                    // In this case, mContext is not used, so operation will succeed even if mContext == null
                    book = new Book(location, name, mCustomSerializers); 
                }
                mBookMap.put(key, book);
            }
            return book;
        }
    }
pilgr commented 2 years ago

Thanks for pointing this out.

You are right, the getBook(String location, String name) api has been added later after establishing initial API design.

The whole API design is outdated for now and should be redone but that would cause backward compatibility issue with preview version. I don't think developers will be willing to update the code to migrate for the new major version of Paper DB since the library if used, used primary in legacy projects.

phazei commented 1 year ago

since the library if used, used primary in legacy projects.

Why do you say that? I just found this library and I've been looking like crazy for something simple that doesn't require "annotations, factory methods, mandatory class extensions etc". I just finished trying to implement kryo5 myself, and couldn't get it to not need a whole list of classes and then still have issues, tried Moshi too, and a few others. This is the first thing I found that just works. So is it old? Is there something better? Why is this mostly in legacy project? Should I not use it in a brand new project?

pilgr commented 1 year ago

You are right, the library just works and it's okay to use for the new projects as well. Although I don't provide much support and updates these days.

The good replacement for Paper could be Jetpack Datastore. I recall being impressed with this library at some point but haven't used myself in any production code https://developer.android.com/topic/libraries/architecture/datastore.

phazei commented 1 year ago

I'm using Datastore, unfortunately it's picky, I ended up needing to use the Proto version of it and items need to be very explicitly defined with it. I looked to this because I wanted a simple 1 liner ability to file cache chunks of data.

My only concern with your library is Java 17. Android Studio keeps pestering me to upgrade to Gradle 8.0, but with Kotlin that led to all sorts of annoying issues and this warning and after reading about it, and trying to work around it all, I just gave up and stayed at Grade 7.5. But it seems there's a push to move everything to Java 17. So I figure the next time I try when a more stable version is out, the issues I saw on this plugin with Java 17 could eventually be an issue as the workaround of a custom serializer seems to entirely negate the point of using this plugin. So hopefully when that becomes the norm, maybe you'll find a chance to fix that bit 😅