Re: [PATCH] binary heap implementation

Assert(!"description of error") is an idiom I've seen more than once,
although I'm not sure I understand why it's not written as Robert says with
the condition in the brackets (or as a print to STDERR followed by abort()
instead).

On 15 November 2012 15:11, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Wed, Nov 14, 2012 at 8:11 AM, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com>
> wrote:
> > Comments? Suggestions?
>
> It could use a run through pgindent. And the header comments are
> separated by a blank line from the functions to which they are
> attached, which is not project style.
>
> + if (heap->size + 1 == heap->space)
> + Assert("binary heap is full");
>
> This doesn't really make any sense. Assert("binary heap is full")
> will never fail, because that expression is a character string which
> is, therefore, not NULL. I think you want Assert(heap->size <
> heap->space). Or you could use (heap->size + 1 >= heap->space)
> elog(LEVEL, message) if there's some reason this needs to be checked
> in non-debug builds.
>
> + {
> + sift_down(heap, i);
> + }
>
> Project style is to omit braces for a single-line body. This comes up
> a few other places as well.
>
> Other than the coding style issues, I think this looks fine. If you
> can fix that up, I believe I could go ahead and commit this, unless
> anyone else objects.
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>