Code tidying

[Patrick Julien]

On October 15, 2003 11:15 am, Boudewijn Rempt wrote:
> On Wednesday 15 October 2003 16:02, Patrick Julien wrote:
>
> <...>
>
> Added some notes to the design document.
>
> > Steal from the Gimp forever?
>
> As far as I'm concerned, no -- on the other hand, there's not much
> literature on developing pixel-painter applications, and the Gimp is about
> the only powerful example available in the wild. But as soon as I know that
> the paint core is solid -- KisPainter and everything under it, and I can
> load brushes, preferably dynamic, and perhaps fix some issues with the
> userinterface, then I'm going to do the things that _do_ interest me. From
> that point, either people cry 'Stop! Halt! You're messing up our perfectly
> fine
> photoshop-clone!', or Krita won't have much similarity to the gimp anymore.
> Of course, it's quite possible that I will fail horribly before I'm even
> half-way there. Or that you will tell me to please stop messing with things
> I don't understand...

No, you misunderstand, I simply did not understand what you meant from that 
comment in your text.

>
> > Code wise:
> >
> > You should use Q_INT32 instead of int.
>
> Changed.
>
> > don't use //, if you want to comment out a large section of code, use #if
> > 0
>
> Why? I rather disliked the complaints of the compiler about unused
> variables, so I commented the code out.

Again, you misunderstand, if you use #if 0, you won't have the compiler 
warnings either.

#if 0
void unusedMethod();
#endif

>
> > You actually have a huge memory leak in your new drawPolyline method.
> > Check out the destructor of KisPixelData.
>
> I though that I didn't need to delete that, because of the shared pointer
> -- did I misunderstand that bit? But don't tease: is the right solution to
> set KisPixelData.owner to true?

That is correct.

>
> > Adding // ??? doesn't really do anything but add clout to the code.
>
> I think you mean clutter, but well, here I have to disagree. That's a
> marker for me to remind me to add comments when I know what that bit is
> about. It flags me that this needs my attention.

Perhaps, but this can easily be kept local to your tree.

>
> > Yeah, why did you reorder data members in KisView, KisPixelData, etc? 
> > They look awful now (tab vs. spaces) and for no apparent reason either.
>
> Well, in some cases, create a little order out of a long, chaotic list, in
> other cases just in the course of ordinary editing. Adding stuff, removing
> lines, noticing that the indent didn't fit, pressing tab in XEmacs to fix
> what I removed. And sometimes comments spill over to the next line, and
> gives the appearance of reordering, as in kispixeldata.cpp, where the
> actual order hasn't changed. Tabs vs spaces are a bit of a problem, I'm
> afraid I have no easy solution. If you don't mind very greatly -- i.e., if
> this is going to hinder you working, then I'll investigate ways of teaching
> XEmacs to use tabs when working in the krita tree.

Well, it's not about the superiority of one style over another, but 
uniformity, multiple styles in the same source can be very discouraging to 
maintain.

>
> And about kis_view.h; I'll probably ream out that file in its entirety when
> I start working on the user interface.

if you say so :)

>
> > Also, if you want to start renaming classes, i.e. KisToolBrush to
> > ToolBrush, change the file name too.
>
> ??? I don't think I renamed any class as such? Oh, wait -- it _was_ called
> BrushTool, and I have started implementing a new brush, without even
> looking at the old code, and I sort of assumed that it would be
> KisToolBrush, consistent with the filename. So, actually it's even more
> consistent now...

good then

>
> Anyway, new diff at the same place...

I will look at it shortly.  I am done with the code for hiding KisDoc, 
however, I haven't updated my koffice CVS in a while, I'm updating to make 
sure it compiles with the latest, I'm also removing usage of some KDE 
classes/methods that have been deprecated.