EnterpriseyIntranet / nextcloud-API

NextCloud OCS API for Python
GNU General Public License v3.0
27 stars 27 forks source link

Refactor main NextCloud object to not dynamically assign methods #42

Open jasongi opened 5 years ago

jasongi commented 5 years ago

This project has helped me out a lot, however the current way the main object is set out makes it hard to develop. This is because the methods are dynamically allocated to the main class, meaning you can't use IntelliSense to view methods (see screenshot).

Screen Shot 2019-08-06 at 1 16 02 pm

It seems like a possible solution would be to refactor the classes as regular mixins and inherit from them. It also looks like the dynamic setup was also done to stop people using "private" functions with underscore in front of them - I don't think this is really required as python tends to follow the "We're all responsible users" principle.

I'm happy to work on a PR if this is agreeable?

matejak commented 5 years ago

@jasongi Thank you for raising this issue! @danil-topc What do you think of this proposal? Jason assumes that there is a particular intention behind the implementation, but I am not aware of that.

danil-topchiy commented 5 years ago

@matejak @jasongi Yes, probably reason for dynamically methods assignment was to "protect" some internal methods, it was like this before we started working on it. But yes, it's not the best way to do it and using Mixins will make code simpler, especially if it will help using the package. Also there is separate requester for each functionality class, which initialized in that class and takes it's endpoint as a parameter, but it also can be refactored.

matejak commented 5 years ago

Great, so @jasongi there is nothing stopping you from creating a pull request that fixes this!

luffah commented 3 years ago

Note : usage of __slots__ may solve that. https://stackoverflow.com/questions/472000/usage-of-slots

luffah commented 3 years ago

@jasongi

If IntelliSense is somehow intelligent, then it shall at least initialize the Class. Therefore if the class Method is overridden it shall appear.

Try the following trick in NextCloud.py. It assign the methods in the NextCloud class without being usable.

import six

class MetaClient(type):
    def __new__(meta, name, bases, attrs):
        cls = type.__new__(meta, name, bases, attrs)
        for functionality_class in API_WRAPPER_CLASSES:
            for potential_method in dir(functionality_class):
                if not (
                        potential_method.isupper() or
                        potential_method.startswith('_')
                    ):
                    if not callable(getattr(functionality_class, potential_method)):
                        pass
                    else:
                        setattr(cls, potential_method, getattr(
                            functionality_class, potential_method))
        return cls

class NextCloud(object, six.with_metaclass(MetaClient)):
   ....
luffah commented 3 years ago

Note : Using "private" functions with underscore is just a way to avoid code repetition.

jasongi commented 3 years ago

If IntelliSense is somehow intelligent, then it shall at least initialize the Class.

I wouldn't use an IDE that executed code without me asking it to - I doubt any of them do.

luffah commented 3 years ago

Sphinx (doc builder) do it.

In Python , it happens to have docstring dynamically computed.

Check that, and change your IDE if it does works.

jasongi commented 3 years ago

VS Code is the most used IDE on the planet. I don't think I'll be changing my IDE because of a library I was using two years ago abuses language features to generate classes that don't support statically analysing the functions.