madwind / flexget_qbittorrent_mod

flexget qbittorrent删种、辅种 自动签到 插件
MIT License
552 stars 117 forks source link

Make get_details_base() more flexible to handle json & Add digitalcore.club #73

Closed vivodi closed 2 years ago

madwind commented 2 years ago

有几点我不是很明白,我对python也不是很熟悉

  1. 为什么把类里的方法提取到类外面去定义,看起来它也不会成为一个提供调用的公用方法,是python的设计规范吗
  2. 对于 digitalcore.pyhandle_user_id的意义是什么,当前我对正则分组的匹配基本都是匹配到了默认取 第1个分组 ,如果需要更复杂的匹配也许会采用传递 (regex,index)元组的方式选择分组
  3. handle_details 是为了更方便从json提取数据吗?但是需要从多个detail_sources中获取数据的时候,用json反而会增加处理难度,因为需要自己拼接json
vivodi commented 2 years ago
  1. 是的,这是Google Python Style Guide中的规定:”除非为了与现有库中定义的 API 集成而被迫使用staticmethod。 改为编写模块级函数。“你可以查看下方链接:

    Never use staticmethod unless forced to in order to integrate with an API defined in an existing library. Write a module level function instead.

    因为get_details_base()目前太长了,所以我就将一些代码提到了函数中,从而让get_details_base()更清晰明了。

  2. 目前没有意义。本来是为了在“id":1234,"username":abc这样的文本中匹配到1234/abc,从而可以访问链接example/userprofile/1234/abc,但是后来发现用户名不是必要的,所以默认取第1个分组就可以了。只是已经写了handle_user_id(),如果以后有需要的站点就可以这样用,所以保留了下来。你可以说一下你的修改意见。
  3. 是的,是为了将details_text通过json.loads(details_text)直接解析为json。多个detail_sources的情况因为没有需求所以暂时没有考虑。你可以说一下你的修改意见。
vivodi commented 2 years ago

目前milkie.ccdigitalcore.club都是通过json交互数据,所以希望能直接处理json数据,而不是用正则。

madwind commented 2 years ago
  1. 我个人感觉是比较奇怪,这样设计,子类也就无法继承这些函数,而这些函数只有类本身会调用他们,脱离了类也没有意义。我想如果继承了一个类,需要调用或者覆盖某些方法,最好还是直接在类内部self解决,而不是去寻找它的哪个父类的模块里提供了这个函数。@staticmethod的意义应该是在于不用实例化就可以直接调用。在前期刚接触python的时候由于idea的提示,没有在方法内使用self的都提示了 Method 'xxx' may be 'static' ,我按照提示添加了不少 @staticmethod,但也仅仅是未了修复warning而已,如果是因为这个原因就要把类里的方法移动到模块里,在我看来是得不偿失的,即使不加 @staticmethod,也不该把他们移动到类外面去。如果你有更明确的理由,可以告诉我。因为我的理解是google推荐避免@staticmethod,而不是把方法都尽量定义到模块里
  2. 如果没有意义,就只保留一种好了,因为看起来实现效果一样,但是却有两个方法,会让人费解
  3. ggn也有一段处理json的,也许你可以定义一个 get_details_by_json 之类的方法,但我看了一下好像他们也没有什么相同之处,对于100个站点里只有这么3个提供了json,我认为直接覆写 get_details 返回更好
vivodi commented 2 years ago
  1. 我认为SiteBase的子类不需要使用_get_user_id(),所以无需考虑继承该函数。定义_get_user_id()是因为get_details_base()太长了,所以将其中的一些代码抽取出来,从而增强get_details_base()代码的可读性。而且我在这个函数的前面加了_,表示它是该模块的内部函数,不应该在其他地方被导入。即使如你第3点所说的增加一个get_details_by_json()之类的函数,也是在SiteBase中增加,而不涉及到它的子类。因此_get_user_id()作为模块内部的函数(除了会被SiteBase中的方法调用外不会有任何地方会使用它)是合理的。

    你可以看一下StackOverflow上的这个回答,其中讲了如果要访问实例则使用普通方法,如果要访问类,则使用@classmethod。如果不需要访问类或实例,则使用函数。很少需要使用@staticmethod,但如果要将函数分组到一个类中(例如,这样它可以被覆盖),即使它不需要访问该类,这种情况下可以使用静态方法。

    _get_user_id()明显不需要使用到类或实例,根据StackOverflow上的回答,只可能使用静态方法或模块函数。又因为如第1段所说的不需要继承该函数,且考虑到Google Python Style Guide中的尽可能不要使用@staticmethod,因此将其编写为模块函数。

    如果你还对这个有疑问,可以看看标准库datetime是怎么做的:"C:\Program Files\Python310\Lib\datetime.py"。里面有很多被类调用的函数都是如我这样放在类的外面,比如:_build_struct_time()_wrap_strftime()_format_offset()等等很多,它们都是只被一个类使用,但是仍然被编写成了模块函数。

    最后,如果你仍然认为改为@staticmethod更好,我将进行修改。

  2. 将按照意见做出更改,移除回调函数支持
  3. 好吧,那就直接覆写 get_details() 返回
vivodi commented 2 years ago

会在晚些时候修改

madwind commented 2 years ago

我看了一下 _build_struct_time()_wrap_strftime()_format_offset() 这些方法都存在两个地方以上的调用,例如_build_struct_time()date datetime 两个类调用。它们应该只是为了代码复用而定义成函数。 我同意代码过长应该拆分,但如果有一天子类想实现自己的 _get_user_id(), 当然你写成了回调,几乎什么都做得到,但如果不是回调函数,子类需要实现自己的_get_user_id() 那它就不得不去修改 get_details_base(), 而不能只在类内部覆写 get_user_id()。 所以我觉得_get_user_id()即使没有使用到实例,它也不具备公用或者提供外部调用的价值,它只是因为代码过长被拆分出来的一个方法,定义在类之外没有任何好处,反而增加了代码重用的难度。 不过我也没说@staticmethod方法更好,我也不明白为什么要定义成 @staticmethod,我之所以使用它完全只是IDE给的提示,删除掉也没有任何影响,我不需要它可以静态调用,只需要它在类内部以便更容易的覆写或者调用

vivodi commented 2 years ago

已按要求作出更改