SQLTeam.com | Weblogs | Forums

Poor Indentation


#1

I just came a cross a Stored Procedure written in our department. The indentation was all over the place. I don't mean just one or 2 lines or sections. I found about 10 different indentation issues, in one Stored Procedure.

What should I do? Or am I making a big deal out of nothing? Just that everyone here is always criticizing the outsourced material.


#2

Do you have House Style coding guidelines? If so then the code needs to confirm to that :slight_smile:

If not then perhaps you need to write the First Draft of some documentation ?

Perhaps format it using a tool like Poor SQL the way that you like it and then "save" the settings as the basis for the Coding Guidelines


#3

Run the code through a free on-line SQL formatting tool -- I too like PoorSQL for this, but there are many other sites as well -- and replace the original code with the formatted code. Then move on.

Trying to standardize indentation / formatting among SQL programmers is like trying to organize ocean waves, it really can't be done.


#4

I think it worth a try ...

... but, either way, "absolutely no formatting at all" is surely unacceptable in any organisation that thinks that it has / aspires to half-decent code?


#5

Q: What do you get when you ask three SQL programmers how to format the code?
A: Four answers.

It's an old joke but it does show how difficult it is to agree on one standard format. Even my own personal preferences evolve over time.
Does the tool you mentioned (or others) allow for each developer to view the same master copy of the code, from source control, in their own format filter? Or does the reformatting take too long to be practical?
I'm thinking that if each developer had their own formatting metadata that the viewer would dynamically reformat the code on the fly. That way everyone could view the code as they like it and it wouldn't matter what the original "master" copy looked like. This would have the advantage that the formatting would always be consistent, when viewed, regardless of how many "different" people and formats modified the script, Also, for shops that wanted to enforce a single style, the same format metadata could be deployed to everyone. In either case, as a standard evolves, the metadata is changed and the code automatically reflects the new standard.


#6

PoorSQL is a web site where you copy the code in an input window, push a button, and get reformatted back in another window. It's basically instantaneous.

I don't know of any plug-in tool for source code control tools or SSMS that does what you're referring to there. Doesn't mean there isn't one, of course, I'm just not familiar with it.

Bottom line, any reasonable formatting is OK with me if all I'm doing is maintaining the code. For unformatted code, it's easy enough to pop it into a formatter. Personally, I think there's more productive work to do than to spend significant time trying to standardize formatting of TSQL.


#7

What a fantastic idea! Store the metadata in a file like a snippet for SSMS or in a database table. This is a great idea for an add-on product to SSMS.

I agree with ScottPletcher that there is more valuable work to be done than formatting but formatting can be important for maintenance and readability and that translates into productivity and money.


#8

It may be because I'm dyslexic, but over the years I have been a keen exponent of Defensive Programming. Of course different programmers will have different views on what works well etc., but any organisation which tolerates unformatted code would worry me as to the quality of the resulting APP. I'm not sure that every developer adopting their own style, provided by a formatting tool, and not having the ability to override that when the need arises is a good idea - although, for sure, it is better than no-style-at-all.

For example, we do some specific things because we think that they help with spotting bugs:

For JOINS we put all the columns for the target table on the left. Typically we are trying to satisfy the PKey of the target table, and thus seeing the columns, vertically aligned, on the left help ensure that all the necessary columns are included. More commonly I see the Target Columns on the right (or switching sides between one JOIN and the next, or even within a single JOIN [sigh!])

SELECT Col1, Col2
FROM Table1 AS A
     JOIN Table2 AS B
         ON B.PKey1 = A.SomeColumn
        AND B.PKey2 = A.OtherColumn
        AND B.PKey3 = 'FixedConstant'

We use the style

    SELECT [AliasName] = ColumnName

because if the [AliasName] is on the right it may well be off the screen, or not easily spotted. We arrange them, neatly indented, so that they align vertically for easy reference and checking. Similarly we put any continuation operator at the start of the line, not the end, so that there is no accidental assumption that something [stretching off the right of the screen] is comma-terminated, and thus a new result column.

SELECT   [MyAlias] = some + long + expression
       + continued + onto + next + line
       , [NextAlias] = FooBar

I very much doubt that a Pretty Printer could do the former, maybe it could do the later, but we permit some leeway as to what is concatenated on a single line, and what is continued onto a fresh line. Often times the actual "trickiness" of the parts of the concatenation, or expression, influence the layout. For example, I don't necessarily want a CASE statement separated onto multiple rows:

    CASE WHEN FooBar IS NULL THEN '' ELSE Some + moderately + complex + expression END

in this example the minimal blank-string is deliberately first (i.e. I would use IS NOT NULL if that was necessary in order to have the minimal expression first), and the major expression second as it immediately indicates that NULL is "ignored" and only non-null values have a detailed expression. If it were the other way around the blank-string might be off the right end of the screen and the developers would not know, without scrolling, whether there were two complex expressions, or whether one of them was trivial - and that will lead, on occasions, to the developer assuming the wrong thing which leads to bugs. The fact that the moderately complex expression may continue off the edge of the screen doesn't matter, enough of it is showing to indicate that there is an expression, which can be scrolled-to-read if necessary. Again, I doubt a formatting tool would turn the expression around to provide that sort of Defensive Programming visual feedback.

Our code styling is within a set of guidelines but there is plenty of personal choice as to where line breaks occur and the like. But I do expect that there are spaces around "=" for example and without doubt a formatting tool could do that part :slight_smile:

 [one]=foo,[oneA]=bar
 [two] = foo, [twoA] = bar

On the first line pressing Control-Arrow skips straight to the end of the line. On the second it stops at each word-break. No doubt some people use a mouse for that type of editing, but we think we are more productive using keyboard controls for most maintenance-editing. And so my list goes on :slight_smile:

Well ... not in SSMS which stops at every possible break character, which makes it useless as a productive editor for skilled keyboard users.

Cost of bug finding & fixing is significant, increasingly so the later in the process that bugs are identified. Over the years I reckon our strike-rate has been exceptionally good; I have plenty of experience as a consultant helping sort out significant problems in APPs and mostly I reckon that the problems could have been avoided if the coding style was not "casual coding". Of course most managers are not interested, because it increases DEV time by, say, 10%. By comparison reducing debugging time by 10% is a huge saving, but difficult to quantify that the saving has been made.

I have no doubt that some folk here will disagree with my formatting style choices, but that is not my point. My point is that developing a set of style guidelines, that works for you, and improves code quality and achieved Defensive Programming is a worthy goal, and I very much doubt that can be provided by a formatting tool. If one exists then Sign Me Up :slight_smile:


#9

Thankyou!

No, we don't have coding guidelines for style. But the things I'm referring to are plain wrong.

We do have some coding guidelines for things that don't relate to style.


#10

Heh... if the code is, in fact, outsourced, be happy that it works. If you want the outsourced code to be formatted, write a spec that they have to meet with monetary penalties if the don't. At the very least, specify that they include a header with the purpose of the code, usage examples, and revision history.

Otherwise, it is what it is and the company has gotten what it paid for.


#11

I meant that this is the code from our department that isn't outsourced! And yet others in our department criticize the outsourced code. But yes some of the outsourced code is unformatted as well.


#12

It's personal preference in my humble opinion.

The last DBA at my company was very highly thought of and apparently a whizz-kid and yet he insisted upon writing his code entirely in lower case and cramming as much as possible onto one line. Horrible. But that's just my opinion.

select * from orders where customerid = '1000' and productid = '22' and orderdate = '2016-04-01' and ordertype = '1'

is, to me much uglier and readable than

SELECT * FROM [Orders]
WHERE [CustomerId] = '1000'
AND [ProductId] = '22'
AND [OrderDate] = '2016-04-01'
AND [OrderType] = '1'