Coding style proposal: double quotes for SQL

Coding style proposal: double quotes for SQL

од David Mudrák -
Број на одговори: 14
Слика од Core developers Слика од Documentation writers Слика од Moodle HQ Слика од Particularly helpful Moodlers Слика од Peer reviewers Слика од Plugin developers Слика од Plugins guardians Слика од Testers Слика од Translators
This is a proposal for adding the following rule into the Development:Coding_style.

Use double quotes for SQL statements, database table names and database field names. This does not influence functionality but with a syntax highlighting properly set, this makes the code more readable.

See the attached screenshot for an example of such highlighting.

Прилог sql-highlight.png
Просек на рејтинзи: -
Во одговорот до David Mudrák

Re: Coding style proposal: double quotes for SQL

од Ravishankar Somasundaram -
David ++


I too have felt the same , and have been coding till now in this manner,
it also makes things easier for me, instead of starting a single quote and then double quote inside for making interpolation of variables,i just use a double quote at the start and then end it with the same,many consider that as a bad practice but no body is justifying why.

This will work for single level variable transformations ex:

echo " some text $textdesc , end of text ";


Просек на рејтинзи: -
Во одговорот до David Mudrák

Re: Coding style proposal: double quotes for SQL

од Aaron Barnes -
Hi David,

Moodle 2 is making the switch to prepared statements - using placeholders for data in SQL statements.

This means you will no longer have to frequently append values and double quotes would be superfluous.

Here is some more info about the new database layer:
http://docs.moodle.org/en/Development:DB_layer_2.0

Cheers,
Aaron
Просек на рејтинзи: -
Во одговорот до Aaron Barnes

Re: Coding style proposal: double quotes for SQL

од David Mudrák -
Слика од Core developers Слика од Documentation writers Слика од Moodle HQ Слика од Particularly helpful Moodlers Слика од Peer reviewers Слика од Plugin developers Слика од Plugins guardians Слика од Testers Слика од Translators

Aaron, thanks for the comment.

Yes, I know the new DB layer and I know how the SQL code looks like in 2.0. Also, using variables inside SQL statement has nothing to do with prepared statements but with binding parameters (syntax is the same but the concept is different). Still, there are situations where you use single quotes (') inside statements - to quote static strings, for example. And, there is a quite often case for putting variables into statements, typically the result of $DB->get_in_or_equal() call.

But apparently I have not underlined the key point here. My proposal has nothing to do with variables expansion. The main reason why I proposed is:

  • Petr already uses this syntax for all SQL's he has touched in 2.0. So this is not new at all.
  • syntax highlighting - even this may appear unimportant, it really counts if you need to quickly find something.
This means that double quotes should be also used at places like $DB->get_records() parameters - again for quick spotting of any SQL stuff.


Просек на рејтинзи: -
Во одговорот до David Mudrák

Re: Coding style proposal: double quotes for SQL

од Eloy Lafuente (stronk7) -
Слика од Core developers Слика од Documentation writers Слика од Moodle HQ Слика од Peer reviewers Слика од Plugin developers Слика од Testers
Hi David,

does the double/single quote make any difference really from a syntax highlight perspective?

I think I haven't anything special in my vim configuration causing both quoted strings to be displayed exactly the same.

Just note I'm using php_sql_query = 1 to highlight the SQL keywords differently within those strings but I get both perfectly pink (brown in your example).

In any case, I really think it's a matter of each IDE capabilities how those strings are displayed and that must not be the cause for any change in our coding style.

And, talking about coding... it used to be generally accepted to use single quotes when possible, afaik. It had some (minor I guess) parsing benefits and so on.

Ciao смешко
Прилог quotesandquotes.png
Просек на рејтинзи: -
Во одговорот до Eloy Lafuente (stronk7)

Re: Coding style proposal: double quotes for SQL

од David Mudrák -
Слика од Core developers Слика од Documentation writers Слика од Moodle HQ Слика од Particularly helpful Moodlers Слика од Peer reviewers Слика од Plugin developers Слика од Plugins guardians Слика од Testers Слика од Translators
The proposed solution is suitable for all IDEs that are able to distinguish between single quoted strings and double quoted strings. Anyway, this was just an idea. No press. In fact, this was originally Petr's idea who "forced" me to use this syntax during his review of the Workshop code...
Просек на рејтинзи: -
Во одговорот до David Mudrák

Re: Coding style proposal: double quotes for SQL

од Dan Poltawski -
I really don't like this proposal, for three reasons:

1) I have slight obsessive compulsive disorder when seeing string literals in double quotes. The age old wisdom that, its faster and more memory efficient as php doesn't need to do complicated string parsing makes me feel like i'm boosting performance with every double quoted string converted to single quotes!

2) I don't think we should be modifying the coding guidelines to fix broken syntax highlighting! We surely should be patching the syntax highlighter to do the right thing!

3) In non-syntax highlighted world (or if you don't trust your syntax-highlighter) I think there are readability benefits to distinguishing string literals being properly distinguished.

For example: I'm doing a security audit for SQL injection attacks I have to look a lot more closely at double quoted string to examine the contents of the string deeply to check for variables which might sneak in. With single quoted I don't have to look so deeply..
Просек на рејтинзи: -
Во одговорот до Dan Poltawski

Re: Coding style proposal: double quotes for SQL

од Tim Hunt -
Слика од Core developers Слика од Documentation writers Слика од Particularly helpful Moodlers Слика од Peer reviewers Слика од Plugin developers
I agree with Dan. -1 for David's original proposal.
Просек на рејтинзи: -
Во одговорот до Tim Hunt

Re: Coding style proposal: double quotes for SQL

од Petr Skoda -
Слика од Core developers Слика од Documentation writers Слика од Peer reviewers Слика од Plugin developers
I do not agree with Dan, +1 from me.

Majority of SQL code in 1.9 and HEAD is already in "double quotes". Making it consistent makes sense to me. Converting everything to ' does not seem to be an option here.

Petr
Просек на рејтинзи: -
Во одговорот до Petr Skoda

Re: Coding style proposal: double quotes for SQL

од Mark Johnson -
Слика од Core developers Слика од Particularly helpful Moodlers Слика од Peer reviewers Слика од Plugin developers
Surely making it consistent should be the other way around? All non-SQL strings (and some SQL strings) use single quotes. In the context of the PHP what the string contains doesn't matter, so just because they contian SQL doesn't mean they should be represented differently.
I don't think the coding guidelines should be changed to allow for where they haven't been followed - the code should be brought into line.

Additionally, I don't think that syntax highlighting is a good enough reason for introducing a coding convention, since it's not universal. If you follow the convention of putting SQL keywords in uppercase, then SQL strings stand out amongst all other code, which according to the guidelines should be lowercase.

Просек на рејтинзи: -
Во одговорот до Mark Johnson

Re: Coding style proposal: double quotes for SQL

од Mark Johnson -
Слика од Core developers Слика од Particularly helpful Moodlers Слика од Peer reviewers Слика од Plugin developers
An additional point - since {braces} in a double-quoted string are used for interpolation (is that the right word?), they could be a cause for confusion, as the new database API uses them to enclose table names. For example:

$sql = 'SELECT * FROM {user} WHERE id = 3;';
$sql = "SELECT * FROM {user} WHERE id = 3;";
Would behave the same, but
$sql = 'SELECT * FROM {$table->name} WHERE id = 3;';
$sql = "SELECT * FROM {$table->name} WHERE id = 3;";
wouldn't.

If I saw a double-quoted string containing something in braces I'd immediately assume it was being used for interpolation, and since all SQL strings are to contain them from now on where they AREN'T being used for interpolation I think double-quotes are a bad idea.
Просек на рејтинзи:Useful (1)
Во одговорот до Petr Skoda

Re: Coding style proposal: double quotes for SQL

од Penny Leach -
I agree with Dan and Tim.

I hate double quotes смешко And Mark's point about table placeholders {} makes a lot of sense as a reason to use single quotes as well.

(PS eloy thanks for the vim-tip ;) )
Просек на рејтинзи: -
Во одговорот до Penny Leach

Re: Coding style proposal: double quotes for SQL

од Martín Langhoff -
My much-diluted vote comes in for Eloy, Dan, Tim, Penny and Mark.

Specially with the new SQL table syntax, and the use of placeholders, there is little reason to use doublequotes.

If we're to consolidate our usage of quotes in SQL, it'd make sense to consolidate on single quotes. This will make odd code immediately stand out.
Просек на рејтинзи: -
Во одговорот до David Mudrák

Re: Coding style proposal: double quotes for SQL

од Petr Skoda -
Слика од Core developers Слика од Documentation writers Слика од Peer reviewers Слика од Plugin developers
Great, we have a winner - the single quotes голема насмевка

I think we could wait a bit for the final word from MD and then convert all SQL to this new coding style, any objections? I hope not, patches can not merge due to previous changes anyway.

Petr
Просек на рејтинзи: -
Во одговорот до Petr Skoda

Re: Coding style proposal: double quotes for SQL

од Martin Dougiamas -
Слика од Core developers Слика од Documentation writers Слика од Moodle HQ Слика од Particularly helpful Moodlers Слика од Plugin developers Слика од Testers
Shrug, I really don't think this is too important to standardise, even though I'd probbaly put single quotes in the coding guidelines for new code.

I'd leave old stuff as it is and let it get slowly cleaned up when people edit it.
Просек на рејтинзи: -