[Kst] Policy on asserts, checks, and other defensive programming.
George Staikos
staikos at kde.org
Wed Mar 16 18:44:10 CET 2005
On Wednesday 16 March 2005 12:23, Barth Netterfield wrote:
> -Checking for 0 size pixmaps is good, because on some window managers,
> with some font sizes and icon sizes, the pixmap could end up zero size; and
> we know of a clean way of handling it (don't draw).
Disallowing 0 size pixmaps in use is less work and less impact, and gets
closer to the root of the problem.
> This is always going to be a judgement call, so:
>
> If, based on this policy, a developer decides to put one in **while working
> on the code in question**, let them. (if you add one, please leave a brief
> comment as to why it is there)
>
> If, based on this policy, a developer decides to remove one **while working
> on the code in question**, let them.
This is ping-pong.
> Actual documented bug fixes are always allowed, of course, but to avoid
> sensless squabbling, please be specific about the behaviour you are
> changing when you make the change...
> (eg, prevents a crash when the user .... or speeds up redraws by ....)
>
> Thoughts? Otherwise I will add it to devel docs.
The problem is that the fix should be the -right- fix, not the easiest fix.
The easiest fix saves 10 minutes and creates a mess in the long run. The
right fix takes knowledge of how things are supposed to work, or an email to
the list to ask how, and then means that there are no side effects or hidden
problems. For instance, see: http://bugs.kde.org/show_bug.cgi?id=100863 .
The easy fix is to check if tempfile->dataStream() is null. Then < 1% of
sites would randomly fail, in many cases unnoticeably. The problem becomes
many times more difficult to debug in the future, and the bug is not fixed,
but the crash dialog doesn't show up anymore. This is the wrong way to fix
bugs. (Not that nspluginviewer is well designed in the first place...)
--
George Staikos
KDE Developer http://www.kde.org/
Staikos Computing Services Inc. http://www.staikos.net/
More information about the Kst
mailing list