Re: [PATCH] binary heap implementation
От | Will Crawford |
---|---|
Тема | Re: [PATCH] binary heap implementation |
Дата | |
Msg-id | CAJDxst7COoQd0CMdw9GSMePp=66f7msA=QKgnhoD3TLxyGhtHA@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: [PATCH] binary heap implementation (Robert Haas <robertmhaas@gmail.com>) |
Список | pgsql-hackers |
Assert(!"description of error") is an idiom I've seen more than once, although I'm not sure I understand why it's not writtenas Robert says with the condition in the brackets (or as a print to STDERR followed by abort() instead).<div class="gmail_extra"><br/><br /><div class="gmail_quote">On 15 November 2012 15:11, Robert Haas <span dir="ltr"><<a href="mailto:robertmhaas@gmail.com"target="_blank">robertmhaas@gmail.com</a>></span> wrote:<br /><blockquote class="gmail_quote"style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> On Wed, Nov 14, 2012 at 8:11 AM,Abhijit Menon-Sen <<a href="mailto:ams@2ndquadrant.com">ams@2ndquadrant.com</a>> wrote:<br /> > Comments? Suggestions?<br/><br /> It could use a run through pgindent. And the header comments are<br /> separated by a blank linefrom the functions to which they are<br /> attached, which is not project style.<br /><br /> + if (heap->size+ 1 == heap->space)<br /> + Assert("binary heap is full");<br /><br /> This doesn't reallymake any sense. Assert("binary heap is full")<br /> will never fail, because that expression is a character stringwhich<br /> is, therefore, not NULL. I think you want Assert(heap->size <<br /> heap->space). Or you coulduse (heap->size + 1 >= heap->space)<br /> elog(LEVEL, message) if there's some reason this needs to be checked<br/> in non-debug builds.<br /><br /> + {<br /> + sift_down(heap, i);<br /> + }<br /><br/> Project style is to omit braces for a single-line body. This comes up<br /> a few other places as well.<br /><br/> Other than the coding style issues, I think this looks fine. If you<br /> can fix that up, I believe I could goahead and commit this, unless<br /> anyone else objects.<br /><span class="HOEnZb"><font color="#888888"><br /> --<br />Robert Haas<br /> EnterpriseDB: <a href="http://www.enterprisedb.com" target="_blank">http://www.enterprisedb.com</a><br/> The Enterprise PostgreSQL Company<br /></font></span><div class="HOEnZb"><divclass="h5"><br /><br /> --<br /> Sent via pgsql-hackers mailing list (<a href="mailto:pgsql-hackers@postgresql.org">pgsql-hackers@postgresql.org</a>)<br/> To make changes to your subscription:<br/><a href="http://www.postgresql.org/mailpref/pgsql-hackers" target="_blank">http://www.postgresql.org/mailpref/pgsql-hackers</a><br/></div></div></blockquote></div><br /></div>
В списке pgsql-hackers по дате отправления: