Statement Classification QUERY, DML, DDL, BLOCK, UNPARSED

[manticore-projects]

Introduce a Statement Classification in order to distinguish QUERY, DML, DDL, BLOCK, UNPARSED
--> Each statement now extends either QueryStatement or DMLStatement or DDLStatement or BlockStatement and will return a StatementType accordingly

Introduce streamlined appendTo() method in order to avoid the creation of multiple StringBuilders
--> Each statement implements the method StringBuilder appendTo(StringBilder builder) and so does not need to implement toString() any longer (because that is done in StatementImpl only)

Improve Test Coverage for all Statements

Very sorry for touching many files, but this is an 'all or nothing' thing.

[wumpz]

I don't get the improvement of your DDLStatement and stuff. This isDDL and getType == DDL and the need for additional classes. Since we are on Java 8 why not simply put a simple interface to the classes like

interface DDL {
    default  StatementType getStatementType() {
          return StatementType.DDL;
     }
}

This default stuff even isn't needed.

As well this StringBuffer with appentTo. What's the point?

[wumpz]

Why this type method? Your have your grouping by the class.

[wumpz]

And additional to this type method there is this boolean stuff as well. Why?

[manticore-projects]

The was pure convenience for the end user:

1. the method getStatementType() is useful for switch case statements
2. the methods isFoo() are useful for if else statements.

Example: Imagine a SQL Editor as part of a Web Application, where certain normal users shall be able to execute queries but only Admin users shall be able to execute DDLs and maybe only privileged would run DMLs. In this scenario these methods would become very handy because otherwise you will end-up with a long list like:

if (foo instanceof Select) {
     Select select = (Select) foo;
     ...
} else if (foo instanceof Update) {
    ...
} ...

[manticore-projects]

As well this StringBuffer with appentTo. What's the point?

Good Morning. Thank you for your feedback.
My thought process was like this:

1. At the moment, there is no consistency: half of the Statements and Expressions use StringBuilders already and the other half uses String concatenation
2. This results in initiation and disposal of hundreds of StringBuilders during the deparsing, especially when you use a lot of nested statements and expressions, putting useless load on the GC
3. String Addition += in Loops is still considered bad style and will be called out by many style checkers, because the Compiler won't substitute the concatenation by a StringBuilder
4. Providing an appendTo() method consistently for all Statements and Expressions would eventually allow the Deparser to work with one single StringBuilder only (initiated at the very top).

I do not see any disadvantages of this pattern? toString() still works without any caveats. We just delegate the logic into a special method. You did the same for many other Statements and Expressions already -- just not calling it appendTo() and not applying a systematic pattern yet.

What counter arguments do you see? Why was the String concatenation better and why would we not eliminate all the StringBuilders then instead?

And how would you actually like to design it, if starting from scratch with 15+ years more experience in your bag now?

[manticore-projects]

I don't get the improvement of your DDLStatement and stuff. This isDDL and getType == DDL and the need for additional classes. Since we are on Java 8 why not simply put a simple interface to the classes like

interface DDL {
    default  StatementType getStatementType() {
          return StatementType.DDL;
     }
}

This default stuff even isn't needed.

Enforcing the appendTo() consistently and avoid using toString() with creating new Stringbuilders was the main goal of StatementImpl.

[wumpz]

Could you provide your thoughts on the statement classification and why you do not use an interface?

[wumpz]

I am not quite sure, why you are closing this? Do you want to restart this statement classification in a new PR?

[manticore-projects]

My impressions was, that you do not like it at all because your never responded to it. If something does not catch within 3 weeks it is usually rubbish -- until it becomes relevant 1 year later.

I have salvaged the TEST improvements though and merged them into the 4.2 preparations.

Maybe we can have a discussion on your preferred direction and then re-open for the 4.3 cycle.
My other suggestion regarding the "Validation inside the toString() method" would go and in hand with these Abstract Statement Classes (because we would need to implement `validFor()' and 'invalidFor()' top level also.) But that is just and idea which still needs to ripe.

[wumpz]

Strange, the last comment 13 days ago was from me. Did skip it?