This is the (hopefully) last part of my list of thing I considered as the obvious way to go while refactoring/reviewing code at some point but turned out to be more controversial than I expected. This week we'll probably agree in a more shared and less personal view about these topics. 1) RSpec as a design validator I like the RSpec approach to unit testing and I consider the RSpec tests of a class like the detailed documentation about its behavior. I try to write the RSpec tests relatively early in the process (without reaching the extreme of TDD) because it allows me to validate my design decisions. If I struggle to write meaningful readable RSpec test or I need a lot of non-obvious mocking during the process, then I conclude that my classes are not properly designed. In that case, I sometimes switch to TDD mode, writing the desired RSpec and then fixing my class. 2) Another note about OOP design In our new code there are some things that I consider kind of "design smells". I can be, as always, wrong but I feel like some of our code is misusing OOP as a mere way of encapsulate functionality. Going back to concrete real examples, we had this: fake_probing = Yast::Storage::FakeProbing.new devicegraph = fake_probing.devicegraph factory = Yast::Storage::FakeDeviceFactory.new(devicegraph) factory.load_yaml_file(input_file) yaml_writer = Yast::Storage::YamlWriter.new yaml_writer.write(devicegraph, output_file) The fact that we had a "new" every couple of lines is a design smell to me. The adopted solution was creating class methods, so we can do things like this. fake_probing = Yast::Storage::FakeProbing.new devicegraph = fake_probing.devicegraph Yast::Storage::FakeDeviceFactory.load_yaml_file(devicegraph, input_file) Yast::Storage::YamlWriter.write(devicegraph, output_file) Just a cosmetic fix for an issue that I consider deeper: we are writing good old procedural code dressed up as OO code. Every class has the only responsibility of implementing one step in a process and only one public method to execute that step (manipulating a set of I/O arguments). Is just one example of solutions that are perfectly correct but look quite unfamiliar to a Ruby developer's eyes and are quite hard to express as a set of RSpec behavior-oriented specs. Not saying they are wrong, just that they don't look like "the ruby way". Like this alternative which (although not perfect) is a much more "rubyist" way of writing the same: factory = Yast::Storage::DevicegraphFactory.new devicegraph = factory.create_from_file(input_file) File.open(output_file, 'w') {|f| f.write devicegraph.to_yaml } 3) Write every piece of code as something to maintain This is more related to Scrum than to Ruby or OOP. Since we are using Scrum to create YaST, I expect most PBIs to result in a YaST client that can be invoked using /sbin/yast2 or in a set of RSpecs that will be executed in every "rake test:unit" and used as a reference documentation in the future. I consider tests in the "examples" directory to be dead code. I consider that moving code from /test or /src/clients into /src/examples is the first step to have it broken with nobody ever noticing. A have a similar feeling about code used for debugging. It should be "in the right place" design-wise. Don't think in the debugging code as something temporary. If you are needing it today, you will probably need it (or something very similar) also in the future. Make it part of your design... and use mechanisms that would be useful when that code is executed as an RSpec test or yast client. That's all. Sorry again for the long text. I suck at summarizing. Cheers. -- Ancor González Sosa YaST Team at SUSE Linux GmbH -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org