On Fri, Nov 4, 2016 at 11:30 AM, Maduranga Siriwardena <[email protected]> wrote:
> > > 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. > It can be argued both ways.. Basically more than 90% of the DML statements would be ANSI SQL's. (we may only need to write specific queries when we need to do very limited operations (eg: as you said setting paginations/datetime casting etc). I believe insert/update/delete syntaxes are similar unless a select involves to do a bulk operation. So its easier if we have such queries near to your code. And my opinion is when you do not have a query near to your code block you always know that this is not an ANSI Sql and its a reminder for the developer to do the relevant change in all types. > > 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> > -- *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
