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

Split up methods for different item types into different "providers" #56

Open
jmkd3v opened this issue Dec 5, 2021 · 6 comments
Open

Comments

@jmkd3v
Copy link
Collaborator

jmkd3v commented Dec 5, 2021

Right now, the Client has a lot of methods and is kind of polluted.

Assets:
    get_asset()
    get_base_asset()
Badges:
    get_badge()
    get_base_badge()
Groups:
    get_group()
    get_base_group()
Places:
    get_place()
    get_places()
Plugins:
    get_plugin()
    get_plugins()
Universes:
    get_universe()
    get_universes()
Users:
    get_user()
    get_user_by_username()
    get_users()
    get_users_by_usernames()
    get_base_user()
    get_authenticated_user()
    user_search()

Maybe we could split these up into sub-providers, where each one deals with items of a certain type.

# Current syntax
user = await client.get_user(1)
# Proposed syntax (one of the following)
user = await client.users.get(1)
user = await client.users.by_id(1)
@jmkd3v jmkd3v pinned this issue Dec 6, 2021
@NORXND
Copy link

NORXND commented Dec 6, 2021

I think this is a good idea (if we are not looking at backward compatibility).
Maybe we could also make one method of fetching users and then detect whenever to use ID or username (I'm not really confident about it though)

@nikita-petko
Copy link
Contributor

You still haven't explained what this base system is

@Boegie19
Copy link
Contributor

Boegie19 commented Dec 6, 2021

To my opinion, it does not make sense since it would only make the code longer you need to write to get done what you want to do.

@jmkd3v
Copy link
Collaborator Author

jmkd3v commented Dec 6, 2021

You still haven't explained what this base system is

https://ro.py.jmk.gg/bases/

@Auriosi
Copy link
Contributor

Auriosi commented Dec 7, 2021

I like it, my only concern is backward compatibility

@jmkd3v
Copy link
Collaborator Author

jmkd3v commented Dec 7, 2021

If we were to do this, it would be very clear that the change is hugely breaking

@jmkd3v jmkd3v unpinned this issue Feb 5, 2024
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

5 participants