Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Mino260806 opened this issue Jul 21, 2022 · 4 comments
Open

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

Mino260806 opened this issue Jul 21, 2022 · 4 comments

Comments

@Mino260806
Copy link

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
Copy link
Owner

pilgr commented Jul 21, 2022

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
Copy link

phazei commented May 4, 2023

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
Copy link
Owner

pilgr commented May 4, 2023

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
Copy link

phazei commented May 6, 2023

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 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants