> But you also do read my mails, do you? And I said clearly that it might 
> make a difference in larger functions not that it necessarily betters 
> anything.

but you continue to state that it makes the code cleaner which it clearly

> Example? Ok, here you go, I subsituted an gint which is used for
> a clearly positive range (1; length) by an guint. This is an arbitrary
> example and doesn't give to much of a benefit though the function is
> still simple.
> --- paint-funcs.c.orig        Thu Nov 29 14:17:47 2001
> +++ paint-funcs.c     Tue Dec  4 21:53:49 2001
> @@ -343,7 +343,8 @@
>    gdouble  sigma2;
>    gdouble  l;
>    gint     temp;
> -  gint     i, n;
> +  guint    i;
> +  gint     n;

it gives a small improvement and is probably a valid optimization in
this place. The difference to the changes I reverted is that this is a
very local change. The function is small and the variable is local. It
is easy to decide that using an unsigned integer here does not have
any unwanted side effects.

Your changes however did affect mostly headers. This means the values
are used in a lot of places and it is almost impossible to check them
all. Even if you would check them all, a future change to the code
might introduce a bug because the one who did the change didn't think
about the unsignedness of the variable (which is defined somewhere
else). Since the code is very much in flux right now, a lot of these
changes are going to happen in the future. It is definitely too early
to do such global optimizations and I even vote for never doing
them. Using unsigned variables locally in parts of the GIMP core that
are executed frequently is still a valid option.

> > I haven't said that and you will have noticed that we've added lots of
> > consts to the code recently.
> And this is a good thing. It might break stuff exactly like the
> "unsigned" case but it will lead to better code and is much cleaner.

no, it breaks things in a very different manner: the compiler will now
warns us if we try to modify values that we aren't supposed to modify.
Thus we catch bugs at compile time. The compiler will however not warn
us if we use unsigned variables in a way that might cause the value to
become negative. The result here will be a mis-behaving application
with no indication of where we should look for the problem. In the
worst case the problem only becomes apparent under rare circumstances
and is not even caught during the development process.

> I wouldn't have touched the UI code if it was more seperate from the
> rest.

the directories that contain pure UI code are called gui and widgets.
Is that not simple enough? The point however is not to disallow
optimizations in the UI and allow them everywhere else. Most parts of
the core are unfinished yet and shouldn't be optimized at this stage.

> >  1. Move files into subdirectories that define subsystems and reduce 
> >     the dependencies between these subsystems.
> >  2. Clean up the code so it uses a common coding-style.
> >  3. Make the code bug-free (this should involve writing test suites 
> >     for the various subsystems).
> >  4. Profile it.
> >  5. Optimize it.
> > 
> > Since we aren't even done with parts 1 and 2; can't you understand
> > that we'd prefer not to start with 5 yet?!
> Sure, but the changes were also part of your point 2, and it's never
> to early to work on cleanliness.

we obviously disagree on code cleanliness. IMO the use of unsigned
variables does not make the code any cleaner. It is solely part of an
optimization process and optimization has to come late and should be
applied with lots of care.

Salut, Sven
Gimp-developer mailing list

Reply via email to