Code review comment for lp:~kaaveeacs/drizzle/privatized-members-of-NestedJoin

Revision history for this message
Monty Taylor (mordred) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/14/2011 12:45 PM, Olaf van der Spek wrote:
> Hi Kaaveea ,
>
> On Thu, Apr 14, 2011 at 9:42 PM, Kaaveea C S <email address hidden> wrote:
>> + /*
>> + This constructor serves for creation of NestedJoin instances
>> + */
>> + NestedJoin()
>> + :
>> + join_list(),
>
> Is it necessary to list join_list here?
> Is the constructor needed at all?

Scott Meyers Effective C++ recommendations are to explicitly list all
data members in the initialization list. If you don't list it there,
it's still going to do the constructor, but you may forget that fact.

>> + List<Item> getSjOuterExprList() const
>
> Shouldn't this be returned by const&?

Yes. Yes it should be:

const List<Item>& getSjOuterExprList() const

Monty

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk2ohk8ACgkQ2Jv7/VK1RgEsnwCguHX4lPEdyjhiM5psnmVWRSJK
26kAoMIzM+cJ37vr8h7y0Jhqj+e4mW8r
=iEcT
-----END PGP SIGNATURE-----

« Back to merge proposal