search

liuis / leveldb

Automatically exported from code.google.com/p/leveldb
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

DB::Get API should use reference instead of pointer #134

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
The current DB::Get API is defined as:
virtual Status Get(const ReadOptions& options,
                     const Slice& key, std::string* value) = 0;

However, 'value' is not an optional parameter - it should point to a valid 
std::string. This is not checked by the implementation

It could be considered to use a reference rather than a pointer:
virtual Status Get(const ReadOptions& options,
                     const Slice& key, std::string& value) = 0;

Original issue reported on code.google.com by jvb...@gmail.com on 3 Jan 2013 at 6:36

GoogleCodeExporter commented 9 years ago 
as Alexander Melnikov points out on the list, this will likely not get fixed as 
the Google style guide specifies this way.

see 
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Reference
_Arguments#Reference_Arguments

tl;dr: google style guide uses pointers for out arguments and references for in 
arguments

Original comment by jtolds on 3 Jan 2013 at 9:21

GoogleCodeExporter commented 9 years ago 
I can see your point (reserve pointers for objects that may be NULL).  However 
as others have pointed out, this is following the Google C++ coding style where 
we use the distinction between pointers and const-references to distinguish 
between output and input parameters.

Sorry, we won't be changing the style we are following in this code.

Original comment by san...@google.com on 3 Jan 2013 at 4:47

GoogleCodeExporter commented 9 years ago 
In that case, perhaps the code should at least check that the pointer != NULL

Original comment by jvb...@gmail.com on 3 Jan 2013 at 5:06

GoogleCodeExporter commented 9 years ago 
I always hated that rule in the style guide.

There is a pretty straightforward solution here: use a typedefs. Something 
descriptive like DB::out_value. Then there can always be a flag to tweak the 
typedefs to one's coding style, and it is more descriptive than the stupid pry 
vs. ref convention.

Original comment by cbsm...@gmail.com on 3 Jan 2013 at 6:16