Re: Coding style question - Mailing list pgsql-hackers
From | |
---|---|
Subject | Re: Coding style question |
Date | |
Msg-id | 1162494848.7998.288.camel@sakai.localdomain Whole thread Raw |
In response to | Re: Coding style question (Gregory Stark <stark@enterprisedb.com>) |
Responses |
Re: Coding style question
|
List | pgsql-hackers |
<br /><blockquote type="CITE"><pre> <font color="#000000">The disadvantage of using initializers is that you end up contorting the code</font> <font color="#000000">to allow you to squeeze things into the initializers and it limits what you</font> <font color="#000000">can do later to the code without undoing them.</font> <font color="#000000">For example, if later you find out you have to, say, lock a table before the</font> <font color="#000000">itupdesc initializer then all of the sudden you have to rip out all the</font> <font color="#000000">initializers and rewrite them as assignments after the statement acquiring the</font> <font color="#000000">table lock.</font> </pre></blockquote><br /> Good point (and I can't argue with your example). But, I think initializers also force you todeclare variables in the scope where they are needed. Instead of declaring <i>every</i> variable at the start of the function,it's better to declare them as nested as practical (not as nested as possible, but as nested as practical). Thatmeans, you might write the code like this:<br /><br /><pre> <tt><font color="#000000">static TransactionId</font></tt> <tt><font color="#000000">_bt_check_unique(Relation rel, IndexTuple itup, Relation heapRel,</font></tt> <tt><font color="#000000"> Buffer buf, ScanKey itup_scankey)</font></tt> <tt><font color="#000000">{</font></tt><font color="#000000"> </font> <font color="#000000"> if( lockTable( ... ))</font> <font color="#000000"> {</font> <tt><font color="#000000"> TupleDesc itupdesc = RelationGetDescr(rel);</font></tt> <tt><font color="#000000"> int natts = rel->rd_rel->relnatts;</font></tt> <tt><font color="#000000"> Page page = BufferGetPage(buf);</font></tt> <tt><font color="#000000"> OffsetNumber maxoff = PageGetMaxOffsetNumber(page);</font></tt> <font color="#000000"> ...</font> </pre> The biggest advantage to that style of coding is that you <i>know</i> when each variable goes out of scope. If youdeclare every variable at the start of the function (and you don't initialize it), it can be very difficult to tell atwhich points in the code the variable holds a (meaningful) value. If you declare local variables in nested scopes, youknow that the variable disappears as soon as you see the closing '}'.<br /><br /><blockquote type="CITE"><pre> <font color="#000000">I admit to a certain affinity to lisp-style programming and that does lead to</font> <font color="#000000">me tending to try to use initializers doing lots of work in expressions. But I</font> <font color="#000000">find I usually end up undoing them before I'm finished because I need to</font> <font color="#000000">include a statement or because too much work needs to be done with one</font> <font color="#000000">variable before some other variable can be initialized.</font> <font color="#000000">But including complex expensive function calls in initializers will probably</font> <font color="#000000">only confuse people trying to follow the logic of the code. Including</font> <font color="#000000">_bt_binsrch() as an initializer for example hides the bulk of the work this</font> <font color="#000000">function does.</font> <font color="#000000">People expect initializers to be simple expressions, macro calls, accessor</font> <font color="#000000">functions, and so on. Not to call out to complex functions that implement key</font> <font color="#000000">bits of the function behaviour.</font> </pre></blockquote><br /> Agreed - you can certainly take initialization way too far, I just think we don't take it far enoughin many cases and I'm wondering if there's some advantage that I'm not aware of.<br /><br /> -- Korry<br/><br />
pgsql-hackers by date: