Re: [PATCH 3/3]vbscript: Implemented builtin function CBool

[Jacek Caban]

On 04/17/13 18:18, larmbr zhan wrote:
> +    double tmp;
> +    WCHAR trueW[] = {'T','r','u','e','\0'};
> +    WCHAR falseW[] = {'F','a','l','s','e','\0'};
> +
> +    TRACE("%s\n", debugstr_variant(arg));
> +
> +    assert(args_cnt == 1);
> +
> +    switch(V_VT(arg)) {
> +    case VT_I2:
> +        tmp = V_I2(arg);
> +        break;
> +    case VT_I4:
> +        tmp = V_I4(arg);
> +        break;

I don't think converting to double is a good idea here. Why don't you do
conversions directly to BOOL for all cases?
> +    case VT_R4:
> +        tmp = V_R4(arg);
> +        break;
> +    case VT_R8:
> +        tmp = V_R8(arg);
> +        break;
> +    default:
> +        ERR("Not a numeric vaule: %s\n", debugstr_variant(arg));
> +        return E_FAIL;
> +    }
> +
> +    if (tmp < 0.0 || tmp > 0.0)
> +        str = SysAllocString(trueW);
> +    else
> +        str = SysAllocString(falseW);
> +
> +
> +    return return_bstr(res, str);

Are you sure you want to return string type here? I'd expect a bool
value (of type VT_BOOL) here...
> }
>
> static HRESULT Global_CByte(vbdisp_t *This, VARIANT *arg, unsigned args_cnt, VARIANT *res)
> diff --git a/dlls/vbscript/tests/api.vbs b/dlls/vbscript/tests/api.vbs
> index 6bd4065..9b25e87 100644
> --- a/dlls/vbscript/tests/api.vbs
> +++ b/dlls/vbscript/tests/api.vbs
> @@ -442,4 +442,9 @@ Call ok(CInt(36.50) = 36, "CInt(36.50) = " & CInt(36.50))
>  Call ok(CInt(36.75) = 37, "CInt(36.75) = " & CInt(36.75))
>  Call ok(CInt(300) = 300, "CInt(300) = " & CInt(300))
>  Call ok(CInt(-300) = -300, "CInt(-300) = " & CInt(-300))
> +
> +Call ok(CBool(5) = "True", "CBool(5) = " & CBool(5))
> +Call ok(CBool(0) = "False", "CBool(0) = " & CBool(0))
> +Call ok(CBool(-5) = "True", "CBool(-5) = " & CBool(-5))

...and that's why getVT(...) test would be nice here.

Thanks,
Jacek

[PATCH 3/3]vbscript: Implemented builtin function CBool

[Zhan Jianyu]

On Thu, Apr 18, 2013 at 6:24 PM, Jacek Caban <jacek(a)codeweavers.com> wrote:
> I don't think converting to double is a good idea here. Why don't you do
> conversions directly to BOOL for all cases?

My fisrt thought on this is to convert all type to double, and then at last
  I just do one-time comparision, to avoid comparision in every Case.
> Are you sure you want to return string type here? I'd expect a bool value
> (of type VT_BOOL) here...
>
> }

Oh, I'm misguided by the doc I referred to ... I will fix this.






--

Regards,
Zhan Jianyu