API: OGRLayer::GetExtentInternal has useless 'bForce' parameter
|Reported by:||blarg||Owned by:||blarg|
OGRLayer::GetExtentInternal has a 'bForce' parameter. If it is zero, then (after zeroing out some class members and validating something that looks like a class invariant), the method returns OGRERR_FAILURE to its caller.
The caller should not be depending on private class members, and should not worry about the class' invariants (they are the class's concern, not the caller's). So those steps can be ignored in this analysis.
This means that it's not useful to call this function with bForce == 0. All such a call would do is return the value OGRERR_FAILURE - but the caller could have obtained this value directly; there was no need to call a method on an instance of OGRLayer merely in order to obtain an error code.
Since the function is not useful to call with bForce == 0, all callers of it with bForce == 0 are buggy. The calls should all be replaced by the constant OGRERR_FAILURE. Fixing these bugs in application code is not GDAL's concern, so will not be considered further here.
Since the only valid use of this function is to call it with bForce != 0, bForce should default to non-zero, so that callers can eventually cease passing the pointless int argument.
Happily, bForce is currently the last parameter to the method, so it can be given a default value (which it currently lacks) without causing any backward compatibility problems.
There are several other similarly bizarre APIs in ogrlayer.cpp and ogrsf_frmts.h . It looks as if someone was trying to force all callers to say "no, I'm not calling this API accidentally; I really, really do want it to do what its documentation says it does".
The problem with this idea is that if an application can accidentally call this API, even though it didn't actually want to call it, then that application can also accidentally call it with bForce != 0.
GDAL can't protect application programmers from themselves.