[
https://issues.apache.org/jira/browse/CXF-3145?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12935089#action_12935089
]
Sergey Beryozkin edited comment on CXF-3145 at 11/24/10 4:34 AM:
-----------------------------------------------------------------
Hi Brian, sorry for a delay, I'm a bit overwhelmed at the moment and at times
like these I usually focus on issues I can deal with fast :-)
thanks for a patch. I have to admit I'm slightly hesitant at the moment as to
whether I want to apply it or not.
First, please do run 'mvn -Psetup.eclipse' which should ensure you get all the
formatting right and will make it easier for me to deal with the patch. We
don't usually have the authors tags but we do acknowledge the authors in the
commit emails.
As far as the actual idea about introducing a visitor is concerned.
I do agree that SearchCondition.toSQL will not work in all the cases even when
some complex enough SQL expressions have to be produced. I'd like to keep toSQL
as not deprecated because I feel it will work in 80% well (80% will want SQL on
the output and 80% will work with simple enough queries for toSQL to work
properly) and it is simpler for a user then worry about initiating a visitor
class.
Now, I'm open to changing the SeachCondition interface to support a visitor
pattern, may be for 2.4, as it was difficult to come with the right interface
without having practitioners like yourself trying it.
However, I did expect toSQL() won't work in 100% cases so that is why
SearchCondition#getSearchConditions() exists.
Giving this method and SearchCondition.getType(), one can 'pull' all the
conditions and easily build the custom expression.
Can you please let me know why do you think applying a Visitor pattern is
better ? The 'pull' style kind of appeals better to me given that it is obvious
when the top level condition is checked and thus the start of the expression
like 'SELECT' can be added. As a side note I'd prefer to have 2 methods only
in the Visitor interface, if we were to go for it (one method accepting the
base SearchCondition interface with implementations relying on getType() and an
explicit toExpression())
Thoughts ?
thanks, Sergey
was (Author: sergey_beryozkin):
Hi Brian, sorry for a delay, I'm a bit overwhelmed at the moment and at
times like these I usually focus on issues I can deal with fast :-)
thanks for a patch. I have to admit I'm slightly hesitant at the moment as to
whether I want to apply it or not.
First, please do run 'mvn -Psetup.eclipse' which should ensure you get all the
formatting right and will make it easier for me to deal with the patch. We
don't usually have the authors tags but we do acknowledge the authors in the
commit emails.
As far as the actual idea about introducing a visitor is concerned.
I do agree that SearchCondition.toSQL will not work in all the cases even when
some complex enough SQL expressions have to be produced. I'd like to keep toSQL
as not deprecated because I feel it will work in 80% well (80% will want SQL on
the output and 80% will work with simple enough queries for toSQL to work
properly) and it is simpler for a user then worry about initiating a visitor
class.
Now, I'm open to changing the SeachCondition interface to support a visitor
pattern, may be for 2.4, as it was difficult to come with the right interface
without having practitioners like yourself trying it.
However, I did expect toSQL() won't work in 100% cases so that is why
SearchCondition#getSearchConditions() exists.
Giving this method and SearchCondition.getType(), one can 'pull' all the
conditions and easily build the custom expression.
Can you please let me know why do you think applying a Visitor pattern is
better ? The 'pull' style kind of appeals better to me given that it is obvious
when the top level condition is checked and thus the start of the expression
like 'SELECT' can be added. As a side note I'd prefer to have 2 methods only
in the Visitor interface, if we were to go for it (one method accepting the
base SearchCondition interface with impls telying getType() and an explicit
toExpression())
Thoughts ?
thanks, Sergey
> Refactor toSQL method as visitor pattern
> ----------------------------------------
>
> Key: CXF-3145
> URL: https://issues.apache.org/jira/browse/CXF-3145
> Project: CXF
> Issue Type: Improvement
> Components: JAX-RS
> Affects Versions: 2.3.0
> Reporter: Brian Topping
> Priority: Minor
> Attachments: cxf3145.patch
>
>
> In doing some work with the FIQL parser, I needed to generate a different
> output than SQL. By using a visitor pattern for rendering the SQL, any
> visitor can be applied to the SearchCondition object graph.
> The attached patch provides that refactoring.
> As a part of that, SearchCondition.toSql() is deprecated, but I maintained
> the interface for maximum compatibility.
> There was also a change to SearchCondition.getSearchConditions(). There were
> no callers to that code except test code, and it was set up to return null if
> there was only one condition. That wasn't clear to me, hopefully that change
> is not unreasonable.
> This may not be formatted properly, please adjust to suit.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
