REEK - Piotr Szotkowski

Piotr is a hacker scientist: an assistant professor at Warsaw University of Technology, a Ruby developer at Rebased and a founding member of the Warsaw Hackerspace. When not coding (or talking about coding) he’s a programme coordinator and a coach at Rails Girls Warsaw and organises Warsaw Ruby Users Group.

reek examines Ruby classes, modules and methods and reports their code smells such as control coupling, data clumping, feature envy, utility functions and simulated polymorphism. It might not always be right, but it will make you think about your design decisions and might just make you a better OOP programmer.

Transcript

1. Hacking on r e e k
2. Piotr Szotkowski ( ) chastell Warsaw University of Technology (assistant professor) (cheap science officer) Rebased
3. class Conference def initialize(name, city, lat, lon) @name = name @lat, @lon = lat, lon end def has_an_acronym?(separated = true) if separated @name.match(/\b\P{Upper}{3,}\b/) unless @name.nil? else @name.match(/\P{Upper}{3}/) unless @name.nil? end end end [1]:Conference has no descriptive comment (IrresponsibleModule) [9, 11]:Conference#has_an_acronym? calls @name.nil? 2 times (DuplicateMethodCall) [7]:Conference#has_an_acronym? has boolean parameter 'separated' (BooleanParameter) [8]:Conference#has_an_acronym? is controlled by argument separated (ControlParameter) [9]:Conference#has_an_acronym? performs a nil-check. (NilCheck) [2]:Conference#initialize has unused parameter 'city' (UnusedParameters)
5. class Conference def directions(lat, lon) if @lat and @lon puts "calculating (#{@lat}, #{@lon}) → (#{lat}, #{lon}) directions…" end end def distance(lat, lon) if @lat and @lon puts "old calculations of (#{@lat}, #{@lon}) → (#{lat}, #{lon}) distance…" end end def distance2(lat, lon) if @lat and @lon puts "new calculations of (#{@lat}, #{@lon}) → (#{lat}, #{lon}) distance…" end end end [2, 15, 21, 27]:Conference takes parameters [lat, lon] to 4 methods (DataClump) [16, 22, 28]:Conference tests @lat && @lon at least 3 times (RepeatedConditional) [27]:Conference#distance2 has the name 'distance2' (UncommunicativeMethodName)
6. class Conference def has_excited_attendees?(attendees) excited, indifferent = attendees.partition(&:excited?) excited.count > indifferent.count end end [33]:Conference#has_excited_attendees? doesn't depend on instance state (UtilityFunction)
7. class Conference def has_delighted_attendees?(attendees) delighted, dismayed = attendees.partition do |attendee| attendee.excited? or attendee.likes?(@city) end delighted.count > dismayed.count end end [38]:Conference#has_delighted_attendees? refers to attendee more than self (FeatureEnvy)
9. what is r e e k ? like RuboCop, but for architecture plug it to your Rake task | CI server
10. skip smells via a config file DataClump: max_copies: 2 min_clump_size: 2 LongParameterList: max_params: 3 overrides: initialize: max_params: 5 TooManyInstanceVariables: max_instance_variables: 9 TooManyStatements: exclude: - initialize max_statements: 5 excludes: from C l a s s # m e t h o d to / m e t h /
11. skip smells via code comments # : r e e k : U t i l i t y F u n c t i o n def has_excited_attendees?(attendees) excited, indifferent = attendees.partition(&:excited?) excited.count > indifferent.count end # : r e e k : D u p l i c a t e M e t h o d C a l l : { m a x _ c a l l s : 2 } def has_an_acronym?(separated = true) if separated @name.match(/\b\P{Upper}{3,}\b/) unless @name.nil? else @name.match(/\P{Upper}{3}/) unless @name.nil? end end
12. making r e e k happy with my code taught me the most about OOP in the last year but always be critical of such tools
15. s­expressions class Conference def has_an_acronym?(separated = true) if separated @name.match(/\b\P{Upper}{3,}\b/) unless @name.nil? else @name.match(/\P{Upper}{3}/) unless @name.nil? end end end
16. (class (const nil :Conference) nil (def :has_an_acronym? (args (optarg :separated (true))) (if (lvar :separated) (if (send (ivar :@name) :nil?) nil (send (ivar :@name) :match (regexp (str "\bP{Upper}{3,}\b") (regopt)))) (if (send (ivar :@name) :nil?) nil (send (ivar :@name) :match (regexp (str "P{Upper}{3}") (regopt)))))))
17. (One thing I won’t mention is the profusion of parentheses (which annoy some people). (What the world needs (I think) is not (a Lisp (with fewer parentheses)) but (an English (with more.)))) Brian Hayes, The Semicolon Wars
18. How r e e k works internally [bin/reek] | | | Application (cli/application.rb) + Options (cli/options) | | | ReekCommand (cli/reek_command) with Reporter (cli/report/report) / | \ / | \ / | \ Source Source Source (source/source_code) | | | | | | | | | Examiner Examiner Examiner (examiner) | | | Examiner sets up a: - SourceRepository (source/source_repository) - a WarningCollector (core/warning_collector) The Examiner then goes through each source: - Initializing a SmellRepository (core/smell_repository) - getting the AST from the source - applying a TreeWalker(core/tree_walker) to process this syntax tree given the SmellRepository - finally have that SmellRepository reporting back on the WarningCollector mentioned above | | | In the last step, the reporter from the ReekCommand: - gathers all the warnings from the collectors of all Examiners (as you can see here https://github.com/troessner/reek/blob/master/lib/reek/cli/r - outputs them with whatever output format we have chose via the cli options
20. collaboration → compromises naming (e.g., r e e k ) tools (e.g., Cucumber) processes (e.g., squashing, ) gitflow
21. contributing to r e e k should get you started (please edit it if it doesn’t!) don’t hesitate to offer trivial fixes (spelling, better naming ideas, etc.) – we’ll let you know if you’re overdoing it :) …that said: we do care about quality and are a nitpicking bunch C O N T R I B U T I N G . m d
23. we do the trivial
25. we make mistakes and fix them going forward
26. what can you work on implement new smells (e.g., LazyClass) refactor the codebase (e.g., RuboCop | r e e k ) get a feel from docs: make them more user­friendly docs: good examples of smells and fixes docs: redo graph recently closed PRs How r e e k works internally
28. RuboCop’s M e t r i c s / A b c S i z e : 18 → 16
31. thanks! r e e k @chastell talks.chastell.net