persistence@glassfish.java.net

Re: Code changes for issue 465 : Postgresql jdbc client error for setObject and Byte parameter

From: Pramod Gopinath <Pramod.Gopinath_at_Sun.COM>
Date: Thu, 23 Mar 2006 12:04:53 -0800

Hi Tom
  I guess I will use the strategy of 2 returns so that it is consistent
with Toplink coding style.
Do you want me send you these changes again for review or can I just go
ahead and check them in.

Thanks
Pramod

Tom Ware wrote:
> Comments inline
>
> Pramod Gopinath wrote:
>
>> Hi Tom
>> Thanks for your quick reply. My comments are inline.
>>
>> Tom Ware wrote:
>>
>>
>>> Hi Guys,
>>>
>>> A couple of comments.
>>>
>>> 1. Why do we need the parameterBound variable in
>>> DatabasePlatform.setComplexParameterValue(). Can we not just
>>> replace the following:
>>>
>>> parameterBound = false;
>>> }
>>> return parameterBound;
>>>
>>> with the following:
>>>
>>> return false;
>>> }
>>> return true;
>>>
>>>
>> Instead of having multiple return, we prefer to set a boolean value
>> and do a return at the end of the method. This is more of our coding
>> preference.
>> But personally we do not have anything against one approach or the
>> other. If you prefer the above style could change the code to that.
>>
>>
> The TopLink coding style tends to be the one I mention. If you feel
> strongly about your choice, I will not object, but prefer the strategy
> with the two returns.
>
>>
>>
>>> 2. In PostGreSQLPlatform is it possible to change the "instance of"
>>> calls to use our getJDBCType() method. For an example of its' use,
>>> look in DatabasePlatform.setComplexParameterValue(). That method
>>> should be faster than "instance of"
>>>
>>>
>> I was taking a look at the DatabasePlatform.getJDBCType() and had
>> query for you. Lets say I have a Long paramater and want to do a
>> setLong(), this will not work. The getJDBCType() would return
>> Types.INTEGER and the code would then only do a setInt(). Isn't this
>> wrong ?
>> Moreover the code as currently written has been tested extensively on
>> the cmp 2.x code base. It has not shown up as a hotpoint in an of the
>> performance profiling activities. Hence we are more comfortable with
>> the code.
>>
>>
>>
> You make an excellent point. Go ahead and continue with the "instance
> of" strategy.
>
> -Tom