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. > > 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
_______________________________________________ Dev mailing list [email protected] http://wso2.org/cgi-bin/mailman/listinfo/dev
