neue Hackordnung
This discussion is connected to the gimp-developer-list.gnome.org mailing list which is provided by the GIMP developers and not related to gimpusers.com.
This is a read-only list on gimpusers.com so this discussion thread is read-only, too.
neue Hackordnung | gg@catking.net | 26 Nov 15:41 |
neue Hackordnung | Sven Neumann | 26 Nov 22:55 |
neue Hackordnung | William Skaggs | 26 Nov 18:04 |
neue Hackordnung | Martin Nordholts | 27 Nov 19:59 |
neue Hackordnung
Hi,
the GIMP is intended to be a collaborative project and I often see comments lamenting the lack of contributions from outside the core team.
I think one cause of this is the accessibility of the code.
I have made some minor contributions with the time I have been able to contribute but 95% of that time and effort has been largely wasted trying to get to grips with illegible, cryptic code.
Now this is my personal experience that I'm feeding back not a personal attack on anyone's individual work so let's avoid defensive rebutals and flames. I may be alone in this opinion or there may be may others who have looked at contributing and just dropped the idea as being too much work. I could have achieved a lot more if the code was more maintainable.
The state of the code base is probably typical of a large project that has evolved and grown over time from a small personal project. That's just the way it is , what I am hoping to put forward are some suggestions to improve coding style that will eventually open the possibility of getting more contributions.
Right , that's the preamble, here's the crux of it. Again anything I cite as an example is just that , not an attack on anyone who may recognise it as thier code.
MAKE CODE READABLE:
please, make code readable with explicit variable names and named constants. Lines like the following are very economical on file size but very wasteful on human time for anyone who did not write the code:
dv = du + w[i];
looks like it came straight of a page of math text book on matrix algebra (and probably did). But once out of that context it becomes meaningless.
We might as well be working in assemble not in a high level language. Now I have to start mentally decoding what the whole function ( more likely the whole file ) is doing and maintain a mental note of what all these anonimous variables represent.
delta_x = delta_y + ceoff[i];
with code like that I dont have to read the file to have half an idea what the line about.
Another classic which seems to purvey a whole rafts of gimp core code it a variable called "byte".
Oddly enough it's not even a byte length variable it's a gint. Sometimes it's called bytes or even int and by the time I've chased it back through about 10 function calles spread over 3 files I find that in fact it refers to PixelRegion.bytes_per_pixel or similar.
Imagine the time that wastes for someone new to the code , it's like a paper chase to find the meaning of one variable that could have had a clear explicit name I could have understood in the line in which it occured. That one was a particualarly devious one to trace but the situation somewhat typical.
[quote="Hackordnung"]One goal of GIMP development is to make the GIMP code
more readable
and understandable. Please help us to achieve this goal by cleaning up
the present code and make sure that all new code follows the coding
guidelines. [/quote]
I recently submitted a patch were I had cleaned up the code and renamed this bytes_pp , this oddly met with some resistance and finally got accepted as bpp. Fine it's a great improvement, although I was a bit dismayed that this sort of change needs debating.
I admit I have written a lot of terse code like this myself in the past but only in one-man projects, this really does not fit in a open-source context, especially if the project is looking for outside help.
ALLOW LONGER LINES:
Longer names make longer lines. Especially if comments are added. Another patch I submitted recently got the comment truncated to the point were it was misleading rather then helpful just because it went a few chars over the manditory line limit.
I think the rigid coding guide of 80 chars should be re-evaluted.
I started programming on punch cards in around 1980, IIRC we had 80 char lines even then, a quarter century ago. My first home computer that displayed on a tele had 80 chars. Do we still need to be that conservative? I'm against obseleting older kit unnecesarily but does anyone working on coding and running gimp not have the width to display more than 80 chars? Maybe 120 chars? Comments?
SUMMARY:
In short anyone wishing to contribute will have finite time available. If code readablility is an obstacle many will not get invovled. The time available to those that do will be largely wasted decrypting the code before being productive.
If the idea gets a favourable reception I will submit a short text for inclusion in Hackordnung advising on readability and variable naming. Current guidelines seem mainly restricted to defining format.
regards, gg
neue Hackordnung
GG wrote: [******]
I agree that it would be helpful to have more guidance for new contributors, but my own experience is that most of my trouble has come from trying to understand the high-level structure -- the overall architecture, the core objects and the principles for using them. The low-level thing that has caused me the most difficulty is signals -- knowing when one is going to be emitted and what is going to happen as a result -- because this is often not apparent from the source code.
On the question of longer lines, I disagree. For comments it doesn't really matter, but I find consecutive long lines of code extremely difficult to read. Also I often work on code with multiple Emacs windows open side by side, and in that scenario even with a good modern monitor it creates difficulties if the code is wide.
There is no question that it is helpful to use good variable names, and lots of the "old" code could definitely be improved in this respect. People's opinions about specifics may vary, though. I myself think "bytes" is fine if it is used consistently with always the same meaning, and "bpp" is fine, but "bytes_pp" impairs readability.
I also think that, strategically speaking, it is best to focus these sorts of improvements on code that needs maintainance for other reasons, because experience shows that even something as seemingly harmless as renaming variables often causes accidental breakage.
Best wishes,
-- Bill
______________ ______________ ______________ ______________
Sent via the CNPRC Email system at primate.ucdavis.edu
neue Hackordnung
Hi,
On Sun, 2006-11-26 at 15:41 +0100, gg@catking.net wrote:
please, make code readable with explicit variable names and named constants. Lines like the following are very economical on file size but very wasteful on human time for anyone who did not write the code:
dv = du + w[i];
looks like it came straight of a page of math text book on matrix algebra (and probably did). But once out of that context it becomes meaningless.
Prepending a variable with d to indicate some sort of difference is well-established. While I agree that a lot of code could be improved, I don't think that delta_v is in any way better than dv.
Fortunately, most of the ugly code is in the lower parts of the core. It's the old code that is going to be rewritten with GEGL sooner or later.
I recently submitted a patch were I had cleaned up the code and renamed this bytes_pp , this oddly met with some resistance and finally got accepted as bpp. Fine it's a great improvement, although I was a bit dismayed that this sort of change needs debating.
Longer names make longer lines. Especially if comments are added. Another patch I submitted recently got the comment truncated to the point were it was misleading rather then helpful just because it went a few chars over the manditory line limit.
I asked you to correct that in your next patch. A comment can easily extend over multiple lines. With longer lines, two editor windows won't fit next to each other any longer.
I think the rigid coding guide of 80 chars should be re-evaluted.
If code gets longer than 80 chars a line, something is wrong anyway. It should probably be rewritten because obviously it's too deeply nested.
If the idea gets a favourable reception I will submit a short text for inclusion in Hackordnung advising on readability and variable naming. Current guidelines seem mainly restricted to defining format.
Not sure if adding anything there along those lines is going to help. Almost all that ugly code that you are refering to is much older than the Hackordnung anyway. The newer parts are all quite readable.
Sven
neue Hackordnung
gg,
I agree with you, in fact I have begun to think about how a complete rewrite using C++ and gtkmm would look, including support for procedural brushes, a good undo/redo system, etc.
From my point of view, C does not fit for a GUI application. Core libraries, like GEGL, makes much more sense to write in C, but for such a complex application as an image editor, C++ fits much better.
I am so far only in the planning stages of this, and this is still very vaporwarish, but I also belive that sooner or later, we need to take a step forward in terms of language expressiveness.
- Martin