Refactoring – Pylint as a guide

First Jenkins pass was quite unpleasant, huge amount of violations. Mainly wrong named variables and lack of doc block.

To make long thing short, after many changes code is almost perfect (according to Pylint). Went from 120 problems to 1 🙂 Refactored code is available at the end of this post.

First pack of problems were code duplications in file BaseDriver. This file is an abstract class and all functions were calling:

raise NotImplementedError

So Pylint said that this all is considered duplication. Not nice, but small change fixed this problem. Just added error message and case closed:

raise NotImplementedError("__init__ not implemented")

Another problem in this file was R0921Abstract class not referenced. Pylint checks if class is referenced – thats ok, but checks only in current file – thats not ok.

I have to use directive to disable this problem in report:

#pylint: disable=R0921

I run check and… it added new information 🙂 This time information that I disabled error reporting. So I added another exclude – don’t display information that I disabled problem reporting. Finally I ended with such directive:

#pylint: disable=I0011,R0921

Next problem was connected with not calling __init__ from classes that inherits BaseDriver. Pylint code W0231

__init__ method from base class %r is not called

Just added

#pylint: disable=I0011,W0231

And problem is gone. I don’t want to call __init__ from abstract class because its throwing an exception.

Next problem was with libraries not existing on Jenkins server. Pylint couldn’t find smbus and RPi.GPIO. I user another directive to disable those errors but not on file but only on line. And I have to add ignoring I0011 problem. Its annoying.

import RPi.GPIO as GPIO #pylint: disable=I0011,F0401
import smbus #pylint: disable=I0011,F0401

Another funny case were with test_lcd.py. It detected lots of duplications and thats ok because thats tests. And tests often do almost the same thing. (Thats my theory :D). So just added directive:

#pylint: skip-file

And it gave me I0013, changed one waring for another information. So I added disable=I0013 and received I0011. After that I gave up and used only skip-file.

To make a short summary.

Pylint is good but only if you know what it is doing. And know what you are doing. Sometimes skipping unnecessary problems is annoying and sometimes impossible. But lets take all good things and ignore bad one 🙂 And good thing is that you know about all kind of mess in your code.

Download code

Advertisements

One comment

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s