Just thought I'd toss out this note
"Refactoring in action" or "How I implemented an RFE in under an hour"
This morning I changed things so that the CMYK color slider mode now has values that range form 0 through 100 instead of 0 through 255. In and of itself that was no big deal, and the ammount of code change needed was minimal. What is interesting, however, are the principles that came into play in doing it. ------------------------
I addressed RFE 994177 "CMYK 0 - 100 not 0 - 255". The first thing was to see if it was needed. Having worked with various media places and artist before, this was a simple one to see the utility of. Not earth-shattering or anything, but something that would make artist and designers working in print a little more comfortable. So, yes, it should be done at some point.
Next question was how much effort would it take. Since I'm in the middle of addressing color issues in general, a lot was fresh in my mind. Then looking into the code I saw it was in very good shape, and prime for reuse. So the level of effort needed should be low.
Now came the fun part. As I went into the code, it was pretty clean and easy to understand. More importantly, it already was setup to mainly separate the UI presentation from the underlying data model itself. In this case, the sliders would need to change depending on the 'mode'. But... the slider widgets were not played with directly, but instead were attached to instances of GtkAdjustment. It turns out that GtkAdjustment is pretty much a 'model' class to represent ranging data in an abstract way.
So... my next step was to look at what needed changing. I first was going to set things up to change the sliders manually when the mode was switched. However, as I moved forward I realized that things were almost clean, and only needed to get set. I just added a 'range limit' member to track which limit the range was using.
Now the really good news was that the functionality I needed to hack on was already moved out to a pair of common functions (get1 and set255). And instead of needing to change those to take a parameter, I just needed to make the code respect the limit already a member of the 'model' class being used (GtkAdjustment). I no longer needed to keep a parameter, and so didn't need ot pass it in or have different functions to do different values.
I did rename those two, just to make things a little clearer on their use. But that was just a minor tweak to make things easier to read down the road.
I did a little find/grep magic to check where things were used and clean them up, but I was done fairly early. Building and testing took up more of the time.
Here is a quick summary of the files I changed:
src/widgets/sp-color-scales.h 1.8 src/widgets/sp-color-scales.cpp 1.13 src/widgets/sp-color-slider.cpp 1.10 src/widgets/sp-color-wheel-selector.cpp 1.9
sp-color-scales.h:
* renamed get1() to getScaled() * renamed set255() to setScaled() * added _rangeLimit (this actually can go away now)
sp-color-scales.cpp:
* set default range limit * use rangelimit in constructing adjustments * changed get1() and set255() to meet the GtkAdjustment limit instead of hardcoded values * set the range limit when switching modes
sp-color-slider.cpp:
* only the get1/set255 function renaming
sp-color-wheel-selector.cpp:
* changed _adjustmentChanged to meet the GtkAdjustment limit instead of a hardcoded value.
======================= Summary:
Things went very fast because of a few things:
* The code was fairly clean and easy to understand to begin with.
* Common functionality was pulled out and centralized into common functions instead of being repeated in-line. (fix it in one place, and it was fixed everywhere)
* There was a nice separation of UI and data model. Functional logic dealt with the data model, while the UI was just a slave to it. (in this case there was the RGBA color value and the GtkAdjustment models in play)
Those combined to make it quite easy to change the UI without negatively impacting the functionality of Inkscape itself. The more we can allow for this type of refactoring, the better Inkscape can be. So I'd like to encourage everyone to try to keep principles like this in mind. Oh, and a big thanks also goes out to all the developers who were in that code before me.