CSXCAD icon indicating copy to clipboard operation
CSXCAD copied to clipboard

Bug in CSRectGrid::IncreaseResolution

Open biergaizi opened this issue 2 months ago • 2 comments

There exists a bug in CSRectGrid::IncreaseResolution, caught in -Wextra by clang: CSRectGrid.cpp:220:18: warning: overlapping comparisons always evaluate to false [-Wtautological-overlap-compare]

The source code reads:

void CSRectGrid::IncreaseResolution(int nu, int factor)
{
        if ((nu<0) || (nu>=GetDimension())) return;
        if ((factor<=1) && (factor>9)) return;
        // ......
}

This is clearly a bug, how can a number be less than 1 and greater than 9 at the same time? It's a logical impossibility, so this check has effectively been disabled for the project's history.

I want to change && to ||, but the question remains: it's logical to reject number less than 1, but why silently rejecting factors greater than 9. Does the code malfunction if the factor is greater than 9? I don't think so. If it's a bad decision by the user, the code should implement that bad decision rather than silently rejecting it. It's more suitable to print a warning message to stderr, or throw a C++ exception, or change the function signature to throw an error code.

biergaizi avatar Dec 06 '25 10:12 biergaizi