add balance statement URL#25
Merged
Merged
Conversation
darai0512
reviewed
May 10, 2024
|
|
||
| class Balance(ListableAPIResource): | ||
| pass | ||
| @classmethod |
Contributor
There was a problem hiding this comment.
静的メソッドの用意、かなり賛成です!
一方で他はインスタンスメソッドなため、ここだけ急に変わるから使い方を覚えてもらったり、後方互換が必要だったり...。
なので両方用意するのはどうでしょう?
class Hoge():
def __init__(self):
self.a = self.__a
@classmethod
def a(cls, param):
print('here' + param)
def __a(self, param):
self.__class__.a(param) # 実装の共通化
Hoge.a('fuga')
Hoge().a('fuga')
instance/classを同名で用意する方法は他にも
https://stackoverflow.com/questions/28237955/same-name-for-classmethod-and-instancemethod
Contributor
Author
There was a problem hiding this comment.
新規の部分にインスタンスメソッド要りますかね。
既存の部分を置き換えるなら後方互換を考えるのは分かりますが、ここは新しく使い始める人しかいないわけで使い方が難しいわけでもないしクラスメソッドだけあれば事足りる気がしましたが
Contributor
There was a problem hiding this comment.
個人的には同名で両方用意するのが上の通り簡単そうだったので、両方あってもいいなと思います。
retrieveとかlistとかの戻りのインスタンスから直接statement_urlsを実行できれば、idを引数指定する必要がなくなる(=誤指定が減る)ので便利さはありそうです。
ここだけ静的メソッドしかなくて、他は両方あるのが(実装側のコードを想像して)歪ってところがネックですが、
いずれにせよ些事ではあるので、方針を貫いてもらえるならどの方針でも。
Contributor
Author
There was a problem hiding this comment.
なるほど。では一応用意しときます
darai0512
approved these changes
May 14, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.