>> In all cases I've checked where HB_ISNIL() is used,
>> it is going to read a POINTER from the parameter, so
>> AFAICS all you have to do is to reverse the iif() results
>> and replace HB_ISNIL() with HB_ISPOINTER() (in the
>> generator).
>>
>> Or, if it can be other than POINTER, you'll have to add
>> new CASE branches in the generator.
>>
>> HB_ISNIL() is only needed if you specifically want to
>> check if a NIL has been passed, it's never useful where
>> you expect any known types which you'll try to read
>> in the next step with one typed hb_par*() call.
>>
>
> I understand your view point but as said before
> I cannot make a jusgement whether it is a pointer or any other type.
> In the final output source it is possible to review what it could be.
I don't understand this. You could as well remove the check
altogether as it currently does nothing besides adding some
unnecessary code. Since you're calling hb_parptr() after
checking for '! HB_ISNIL()', hb_parptr() will do the dirty work
and return real pointer if passed value was POINTER type, and
NULL if it wasn't. So in this case HB_ISNIL() is simply
unnecessary code taking speed and CPU time.
> This deals with the default parameters and I see no harm against this
> checking.
HB_ISNIL() is wrong to check for default parameters. It's just
the C version of wrong parameter checking in .prg code
(DEFAULT x TO y -> while the correct would be to do
'IF ! IS<type>( x ); x := y; ENDIF'). But in .prg code this
will cause RTE, not a potential GPF.
There are four cases involving parameter passing:
1) Valid parameter can be of one type:
local = HB_IS<type>( n ) ? hb_par<type>( n ) : <default>
2) Valid parameter can be of multiple valid types:
local = HB_IS<type1>( n ) ? hb_par<type1>( n ) :
( HB_IS<typen>( n ) ? hb_par<typen>( n ) : default );
3) We accept any type except NIL
if( HB_ISNIL( n ) ) local = default;
<code which handles all possible types>
4) We accept any type including NIL
if( hb_pcount() < x ) local = default;
<code which handles all possible types including NIL>
HB_ISNIL() useful to set the default value only in case 3).
Some other constructs which I used to see in code:
1) local = HB_ISNIL( 1 ) ? "default" : hb_parc( 1 )
This will GPF if a non-string and non-NIL (f.e. a number) is passed
Easily possible if user mistypes a comma when making the call.
Correctly this should be:
local = HB_ISCHAR( 1 ) : hb_parc( 1 ) : "default"
2) local = HB_ISNIL( 1 ) ? "" : hb_parc( 1 )
Besides above GPF problem, here HB_ISNIL() is superfluous since the
same can be solved with: local = hb_parcx( 1 ), which will return ""
when non-string is passed.
Correctly this should be:
local = hb_parcx( 1 );
3) local = HB_ISNIL( 1 ) ? NULL : hb_parc( 1 )
There is a variation when HB_ISCHAR() is used instead of HB_ISNIL().
This is just useless code, much optimal to use:
local = hb_parc( 1 )
4) local = HB_ISNIL( 1 ) ? NULL : hb_parptr( 1 )
There is a variation when HB_ISPOINTER() is used instead of HB_ISNIL().
This is just useless code, much optimal to use:
local = hb_parptr( 1 )
5) local = HB_ISNIL( 1 ) ? 100 : hb_parni( 1 )
This won't GPF, but will give different defaults depending on passed
type, it will either be 100 (when NIL is passed) and 0 when any non-numeric
type is passed.
Correctly this should be:
local = HB_ISNUM( 1 ) ? hb_parni( 1 ) : 100
6) local = HB_ISNIL( 1 ) ? 0 : hb_parni( 1 )
This won't GPF, but it's much shorter and faster to use:
local = hb_parni( 1 ). I sometimes add it as comment that we're
using the zero value as intended default.
F.e. in GTWVG code, there are 136 occurrences of HB_ISNIL(),
most of the fall into above usages and could be enhanced / fixed.
Unfortunately I see above constructs many times in 3rd party
code, while with the same amount of code, many times with
even less (also causing speed up and code size reduction), it
could be solved correctly.
[ I saw your message while writing this. I'm sending it anyway
for future reference. ]
Brgds,
Viktor
_______________________________________________
Harbour mailing list
[email protected]
http://lists.harbour-project.org/mailman/listinfo/harbour
