2

I recently used pycharm and sonarqube to review my code. Both tools suggested to update a lot of methods such as get_success_url which are not using self in their body to use the @staticmethod decorator.

I totally understand why but I feel it is almost pointless. Regarding performance, I'm not sure it will be that helpful. Moreover, these are often overridden django methods. Can I safely use that decorator or it it so pointless it's not worth the time to update?

jonrsharpe
  • 115,751
  • 26
  • 228
  • 437
abrunet
  • 1,122
  • 17
  • 31
  • 1
    Personally I think this might be the only inspection that I have disabled because I feel like this way too. – Wtower Oct 09 '15 at 09:46
  • The [methods you're overriding](https://github.com/django/django/blob/master/django/views/generic/edit.py#L63) do use `self` - are you not calling the base implementation? Why not? – jonrsharpe Oct 09 '15 at 09:52
  • what if I have something like `def get_success_url(): return reverse_lazy('admin_team_list')`? (Yep, I know I could directly specify with `success_url = ...` – abrunet Oct 09 '15 at 09:55
  • One benefit of using static methods is that it tells the reader of your code that you've actually thought about the design, and that `self` isn't used because it isn't needed, rather than accidentally missed out, or intended to be introduced in a subclass. I'd guess that static methods will be slightly more efficient at runtime as they don't need a reference to the instance (and `timeit` agrees with me), and in terms of safety if you're not using `self` then switching won't be a problem. – jonrsharpe Oct 09 '15 at 09:59
  • Possible duplicate of [Why does pycharm propose to change method to static](http://stackoverflow.com/questions/23554872/why-does-pycharm-propose-to-change-method-to-static) – Sayse Oct 09 '15 at 10:00
  • Also, [Static methods in Python?](http://stackoverflow.com/q/735975/1324033) – Sayse Oct 09 '15 at 10:01
  • Well not exactly, I wasnt asking why but if it's worth it or not. I guess @jonrsharpe is absolutly right. If you write it down, i'll put your answer as accepted. – abrunet Oct 09 '15 at 10:04

1 Answers1

2
  • In terms of performance, a trivial example suggests that calling a static method is slightly more efficient than calling an instance method (which you'd expect, since it doesn't have to pass the instance reference around):

    >>> class Test(object):
        def method(self):
            pass
        @staticmethod
        def static_method():
            pass
    
    
    >>> import timeit
    >>> setup = 'from __main__ import Test;t = Test()'
    >>> timeit.timeit('t.method()', setup=setup)
    0.1694500121891134
    >>> timeit.timeit('t.static_method()', setup=setup)
    0.14622945482461702
    
  • In terms of safety, given that your method doesn't actually refer to the instance (or the overridden method implementation) switching to @staticmethod will make no difference.

  • In terms of readability, it tells people looking at your code that you've actually considered the design and that self isn't used within the body on purpose, not accidentally.

jonrsharpe
  • 115,751
  • 26
  • 228
  • 437
  • 1
    I understand your point about `staticmethod` showing that you have considered the design, but I think it might make the code more confusing later on. Take the example of `get_success_url`. It might not require `self` at the moment, but what if I want to change it to use `self.request` later on. If I see the `staticmethod` decorator, I don't immediately know whether the method *has* to be a staticmethod (e.g. something calls `MyModel.get_success_url()`, or if it's just a static method at the moment because it doesn't use `self`. – Alasdair Oct 09 '15 at 10:15
  • 1
    @Alasdair that's a good point; static (and class) methods can be called on the class or the instance, whereas instance methods can only be called on the instance. But where do you draw the line with YAGNI? – jonrsharpe Oct 09 '15 at 10:17