On Fri, Nov 4, 2016 at 11:01 AM, Lahiru Cooray <[email protected]> wrote:
> > > On Thu, Nov 3, 2016 at 10:57 AM, Maduranga Siriwardena <[email protected] > > wrote: > >> 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? >> > Agreed with RuwanA. -1 for the current approach. Giving the control to > modify the DML statements as a configuration is not a good practise unless > if it's an extension point. Is there any reason why we are currently doing > this? > > >> And when there are different SQL queries for different database types, >> readability of the method is worsen in my opinion. >> > If the query is an ANSI SQL statement, we could have it closer to the code > and if not +1 to use a factory method separate the statements DB type wise. > Having 2 patterns at the same time is not a good practice in my opinion. We have to stick to one of the methods throughout the whole code base. The only place I have come across that needs different query types (when inserting, updating, deleting or selecting data) is when using the limits. So we may not need worry much about it. > > >> >> 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 >> >> > > > -- > *Lahiru Cooray* > Software Engineer > WSO2, Inc.;http://wso2.com/ > lean.enterprise.middleware > > Mobile: +94 715 654154 > -- 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
