Hi Ruwan,

Having the SQL query in the method itself improves the readability.

However in C4 usercore and C5 user store connector current implementation,
we have the SQL queries in a central place and load it to a map at the time
of the initialization of connector. With this approach one of the
advantages we have gained is we can load the SQL queries from a config file
(may be to support a new database type without changing the code). How can
we do this with the new approach?

And when there are different SQL queries for different database types,
readability of the method is worsen in my opinion.

Thanks,
Maduranga.

On Tue, Nov 1, 2016 at 12:46 PM, Ruwan Abeykoon <[email protected]> wrote:

> Hi Rajith,
> +1
>
> final String insertIdpSql
>
> On Tue, Nov 1, 2016 at 11:33 AM, Rajith Roshan <[email protected]> wrote:
>
>> Hi Ruwan,
>>
>> Since the final variable is not static can we name it using camel case
>> convention[1]. Otherwise check style plugin will give errors.
>>
>> [1] - final String insertIdpSql
>>
>> Thanks!
>> Rajith
>>
>> On Fri, Oct 28, 2016 at 9:41 AM, Afkham Azeez <[email protected]> wrote:
>>
>>> Query should be close to the code. +1 for Ruwan's proposal. Not only for
>>> queries, you should use your judgment when deciding where to place
>>> constants as well. In many cases, it makes sense to keep the constants in
>>> the relevant class where it is used but people have been blindly putting
>>> those into a Constants class.
>>>
>>> On Fri, Oct 28, 2016 at 8:04 AM, Sumedha Rubasinghe <[email protected]>
>>> wrote:
>>>
>>>> I am actually in favour of what Ruwan is suggesting.
>>>>
>>>> 1. When a problem comes we  usually start by looking at the particular
>>>> query. There is no need to look at all queries together.
>>>>
>>>> 2. Having the query close to rest of the jdbc code is easier.
>>>>
>>>> 3. Regardless of where it is, all queries should be formatted for
>>>> readability than putting it as a single liner string
>>>>
>>>> On Oct 28, 2016 7:27 AM, "Selvaratnam Uthaiyashankar" <[email protected]>
>>>> wrote:
>>>>
>>>>> Do we have any requirements to see all queries together? If so,
>>>>> defining in a single place would be easy. However, I am not sure whether 
>>>>> we
>>>>> have used that requirement ever. We normally look at the query local to 
>>>>> the
>>>>> method? If so, I am +1 for Ruwan's proposal. Being local constant to the
>>>>> method seems good.
>>>>>
>>>>> On Thu, Oct 27, 2016 at 9:42 PM, Sagara Gunathunga <[email protected]>
>>>>> wrote:
>>>>>
>>>>>>
>>>>>> Personally I don't have strong preference on one style over other,
>>>>>> what mostly important for me is maintain one style consistently within 
>>>>>> the
>>>>>> product and if possible across the company.
>>>>>>
>>>>>> I'm adding few other people for their opinion.
>>>>>>
>>>>>> Thanks !
>>>>>>
>>>>>> On Thu, Oct 27, 2016 at 5:34 PM, Ruwan Abeykoon <[email protected]>
>>>>>> wrote:
>>>>>>
>>>>>>> Hi Devs,
>>>>>>> We have been following a convention to put Database query string in
>>>>>>> a constant elsewhere in the code when performing DB CRUD operations.
>>>>>>>
>>>>>>> e.g.
>>>>>>>
>>>>>>> try {
>>>>>>>     String sqlStmt = 
>>>>>>> IdPManagementConstants.SQLQueries.GET_IDP_BY_NAME_SQL;
>>>>>>>     prepStmt = dbConnection.prepareStatement(sqlStmt);
>>>>>>>     prepStmt.setInt(1, tenantId);
>>>>>>>     prepStmt.setInt(2, MultitenantConstants.SUPER_TENANT_ID);
>>>>>>>     prepStmt.setString(3, idPName);
>>>>>>>     rs = prepStmt.executeQuery();
>>>>>>>
>>>>>>> ...
>>>>>>>
>>>>>>>
>>>>>>> However I would propose to keep the query in the same location as the 
>>>>>>> SQL parameter binding and result set reading. Shall we make it within 
>>>>>>> local final variable(constant) in  the method itself.
>>>>>>>
>>>>>>> The advantage is that the Query is visible on the same screen as it is 
>>>>>>> being used. This reduces number of screen flips. Also remembering the 
>>>>>>> query at somewhere else is error prone.
>>>>>>>
>>>>>>> This also help easy review and do query optimizations.
>>>>>>>
>>>>>>>
>>>>>>> e.g.
>>>>>>>
>>>>>>> final String INSERT_IDP_SQL = "INSERT INTO IDP (NAME, DISPLAY_NAME, 
>>>>>>> DESCRIPTION) VALUES(?,?,?)";
>>>>>>>
>>>>>>> this.jdbcTemplate.executeUpdate(INSERT_IDP_SQL, (preparedStatement, 
>>>>>>> bean) -> {
>>>>>>>     MetaIdentityProvider metaIdentityProvider = 
>>>>>>> identityProvider.getMetaIdentityProvider();
>>>>>>>     preparedStatement.setString(1, metaIdentityProvider.getName());
>>>>>>>     preparedStatement.setString(2, 
>>>>>>> metaIdentityProvider.getDisplayName());
>>>>>>>     preparedStatement.setString(3, 
>>>>>>> metaIdentityProvider.getDescription());
>>>>>>>
>>>>>>> ...
>>>>>>>
>>>>>>>
>>>>>>> Please note that they are the same in terms of memory use and
>>>>>>> performance wise as compiler make them constants.
>>>>>>> Putting them as constants serve no purpose as the query will never
>>>>>>> be reused in proper designed Data Access layer. We should reuse the 
>>>>>>> code,
>>>>>>> not the query.
>>>>>>>
>>>>>>> WDYT?
>>>>>>>
>>>>>>> --
>>>>>>>
>>>>>>> *Ruwan Abeykoon*
>>>>>>> *Associate Director/Architect**,*
>>>>>>> *WSO2, Inc. http://wso2.com <https://wso2.com/signature> *
>>>>>>> *lean.enterprise.middleware.*
>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Sagara Gunathunga
>>>>>>
>>>>>> Associate Director / Architect; WSO2, Inc.;  http://wso2.com
>>>>>> V.P Apache Web Services;    http://ws.apache.org/
>>>>>> Linkedin; http://www.linkedin.com/in/ssagara
>>>>>> Blog ;  http://ssagara.blogspot.com
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> S.Uthaiyashankar
>>>>> VP Engineering
>>>>> WSO2 Inc.
>>>>> http://wso2.com/ - "lean . enterprise . middleware"
>>>>>
>>>>> Phone: +94 774895474
>>>>>
>>>>
>>>
>>>
>>> --
>>> *Afkham Azeez*
>>> Director of Architecture; WSO2, Inc.; http://wso2.com
>>> Member; Apache Software Foundation; http://www.apache.org/
>>> * <http://www.apache.org/>*
>>> *email: **[email protected]* <[email protected]>
>>> * cell: +94 77 3320919 <%2B94%2077%203320919>blog: *
>>> *http://blog.afkham.org* <http://blog.afkham.org>
>>> *twitter: **http://twitter.com/afkham_azeez*
>>> <http://twitter.com/afkham_azeez>
>>> *linked-in: **http://lk.linkedin.com/in/afkhamazeez
>>> <http://lk.linkedin.com/in/afkhamazeez>*
>>>
>>> *Lean . Enterprise . Middleware*
>>>
>>> _______________________________________________
>>> Dev mailing list
>>> [email protected]
>>> http://wso2.org/cgi-bin/mailman/listinfo/dev
>>>
>>>
>>
>>
>> --
>> Rajith Roshan
>> Software Engineer, WSO2 Inc.
>> Mobile: +94-72-642-8350 <%2B94-71-554-8430>
>>
>
>
>
> --
>
> *Ruwan Abeykoon*
> *Associate Director/Architect**,*
> *WSO2, Inc. http://wso2.com <https://wso2.com/signature> *
> *lean.enterprise.middleware.*
>
>
> _______________________________________________
> Dev mailing list
> [email protected]
> http://wso2.org/cgi-bin/mailman/listinfo/dev
>
>


-- 
Maduranga Siriwardena
Software Engineer
WSO2 Inc; http://wso2.com/

Email: [email protected]
Mobile: +94718990591
Blog: http://madurangasblogs.blogspot.com/
<http://wso2.com/signature>
_______________________________________________
Dev mailing list
[email protected]
http://wso2.org/cgi-bin/mailman/listinfo/dev

Reply via email to