views:

359

answers:

4

I have a very data-centric application, written in Python / PyQt. I'm planning to do some refactoring to really separate the UI from the core, mainly because there aren't any real tests in place yet, and that clearly has to change.

There is some separation already, and I think I've done quite a few things the right way, but it's far from perfect. Two examples, to show you what kind of things are bothering me:

  • When the user right-clicks on the representation of a data object, the context menu that pops up is created by the data object, although this data object (essentially the ORM representation of a database row) should clearly have nothing to do with the UI.

  • When something is written to the database, but the write fails (e.g. because the database record is locked by a different user), the classical "retry / abort" message box is presented to the user. This dialog is created by the data provider*, although the provider should obviously not have any UI functionality. Clearly, the provider can instead raise an exception or otherwise indicate failure, and the UI can catch that and act accordingly.

    * that's the word I use for the object that basically represents a database table and mediates between its data objects and the database engine; I'm not sure whether that's what is usually called a "provider"

I don't have experience with testing, so I don't easily "feel" testability problems or the like, but before I get into that, some reorganizing has to be done.

There is no complicated business logic involved (it's mainly just CRUD, yes, even without the D), and this would be much more reorganizing than rewriting, so I'm not really concerned about introducing regression bugs like discussed in this question.

My plan is to start refactoring with the idea in mind that the UI part could easily be ripped out to be replaced by, say, a web frontend or a text-based interface instead of the Qt interface. On the other hand, Qt itself would still be used by the core, because the signal/slot mechanism is used in quite a few places, e.g. data objects emit a changed signal to indicate, well, you know what.

So, my question: Is that a feasible approach to increase testability and maintainability? Any other remarks, especially with Python in mind?

+7  A: 

If you have not done so already, read "Working Effectively with Legacy Code" by Michael Feathers - it deals with exactly this sort of situation, and offers a wealth of techniques for dealing with it.

One key point he makes is to try and get some tests in place before refactoring. Since it is not suitable for unit tests, try to get some end-to-end tests in place. I believe that Qt has its own testing framework for driving the GUI, so add tests that manipulate the GUI against a known database and verifies the result. As you clean up the code you can replace or augment the end-to-end tests with unit tests.

Dave Kirby
I haven't read it yet, but I've seen it mentioned in quite a few places, so I'll definitely take a look at it. Thanks.
balpha
+2  A: 

If you want to extract all GUI parts of your application from all the other parts in order to tests all your application, you should use the Model-View-Presenter: You can find some explanation here and here.

With this model, all your services of your application uses the presenters whereas only the user can interact directly with the views (GUI parts). The presenters are managing the views from the application. You will have a GUI part independent from your application in the case you want to modify the GUI framework. The only thing you will have to modify are the presenters and the views themselves.

For the GUI tests you want, you just have to write unit tests for presenters. If you want to test GUI uses, you need to create integration tests.

Hope that helps!

Patrice Bernassola
Yes, I'm aware of that pattern and that's more or less were I'm heading. Actually, to some extend that's what I've done already. So I take your answer as a hint that I'm taking the right direction :-) Thanks
balpha
You're welcome balpha. I think this is the best way for your app to be more testable.
Patrice Bernassola
+1  A: 

I have done refactoring for large legacy code aiming UI/backend separation before. It's fun and rewarding.

/praise ;)

Whatever pattern one calls it or be it part of MVC it's invaluable to have a very clear API layer. If possible you may route all the UI requests through a dispatcher that would offer you greater control over UI<->Logic communication eg. implementing caching, auth etc.

To visualize:

[QT Frontend]
[CLIs]             <=======> [Dispatcher] <=> [API] <==> [Core/Model]
[SOAP/XMPRPC/Json]
[API Test Suite]

This way

  • it's easier to add test suite to test your APIs.
  • Also it makes adding more UIs uniform way and easier.
  • API Documentation: Say if you want to document and expose APIs though some RPC interface, it' easier to generate API documentation. If somebody don't agree with importance of API documentation can always look at Twitter API and it's success.
  • You can quickly import API layer to python shell and play with it

API designing can happen well before you start coding for API layer. Depending on the application you might want to take help of packages like zinterfaces. This is general approach I take even while writing very small apps and it has never failed for me.

Do look at

One distinct advantage of this approach is after you have API layer and new UI, you can now go back to legacy code and fix/refactor it in perhaps smaller steps.

Other suggestions is to have your testing suite ready. See interstar's advice at http://stackoverflow.com/questions/66361/what-are-the-first-tasks-for-implementing-unit-testing-in-brownfield-applications .

Shekhar
Now that's an interesting idea, love it! You don't happen to have any sources or tipps concerning the dispatcher? I.e. different implementation possibilities, possible caveats, experiences, etc.?
balpha
Dispatcher would pass all the API calls to all the subsystems like cache manager, validator, URL mapper... So dispatcher passes the API call, context, arguments to each of the subsystems (depending on certain rules) and finally invoke the API if required. We initially tried to have generic dispatcher design to allow registration of subsystems but later on settled for dispatcher that has knowledge of how to deal with subsystems. Unfortunately this framework for API centric applications is a closed source but effect is something like a stack of decorators for every API but in a implicit way.
Shekhar
Also determining and passing context was played very important role.
Shekhar
+1  A: 

One point, not mentioned so far and not really answering the question, but very important: Wherever possible you should put in the test now, before you start refactoring. The main point of test is that to detect if you break something.

Refactoring is something where it is really valuable to exactly see where the effect of some operation changed and where the same call produces a different result. That's what all this testing is about: You want to see if you break something, you want to see all the unintentional changes.

So, make tests now for all the parts that still should produce the same results after refactoring. Tests aren't for perfect code that will stay the same forever, tests are for the code that needs to change, the code that needs to be modified, the code that will be refactored. Tests are there to make sure your refactoring really does what you intend it to do.

sth
You're right, it hasn't been explicitly mentioned, thanks. It's one of the key points of "Working Effectively with Legacy Code" (which I did read in the meantime), and an interesting approach to this is in the Stack Overflow answer that Shekhar links to in his answer.
balpha