Submitting ByteStream chrome class for Google C++ Readability Review.

content/browser/download/byte_stream.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CONTENT_BROWSER_DOWNLOAD_BYTE_STREAM_H_
 #define CONTENT_BROWSER_DOWNLOAD_BYTE_STREAM_H_
 #pragma once
 
 #include <set>
 #include <utility>
@@ skipping 38 matching lines @@
 // class to prevent registered callbacks from being called after any
 // objects to which they may refer have been destroyed.  It is the
 // responsibility of the callers to avoid use-after-free references.
 // This may be done by any of several mechanisms, including weak
 // pointers, scoped_refptr references, or calling the registration
 // function with a null callback from a destructor.  To enable the null
 // callback strategy, callbacks will not be stored between retrieval and
 // evaluation, so setting a null callback will guarantee that the
 // previous callback will not be executed after setting.
 //
 // Class methods are virtual to allow mocking for tests; these classes

[erikmc, 2012/06/18 19:25:44]

Please include sample usage in the comments.

http://www.corp.google.com/eng/doc/cppguide.xml?showone=Class_Comments#Class_...

[Randy Smith (Not in Mondays), 2012/06/18 20:38:24]

Tug on a string :-}.  This may have added 50% to the number of lines in the
header file.  Let me know what you think.

(Not arguing with the request at all; just amused at the amount of verbiage it
produces.)

[erikmc, 2012/06/21 17:39:44]

Adding useful comment lines at the top of the file are never something to worry
about.  This looks good, and honestly given the complexity required to use this
class, it is great to have a concrete example.

On 2012/06/18 20:38:24, rdsmith wrote:

 // aren't intended to be base classes for other classes.
 class CONTENT_EXPORT ByteStreamWriter {
 public:
   virtual ~ByteStreamWriter() = 0;
 
   // Always adds the data passed into the ByteStream.  Returns true
   // if more data may be added without exceeding the class limit
   // on data.  Takes ownership of |buffer|.
   virtual bool Write(scoped_refptr<net::IOBuffer> buffer,
                      size_t byte_count) = 0;
@@ skipping 47 matching lines @@
 CONTENT_EXPORT void CreateByteStream(
     scoped_refptr<base::SequencedTaskRunner> input_task_runner,
     scoped_refptr<base::SequencedTaskRunner> output_task_runner,
     size_t buffer_size,
     scoped_ptr<ByteStreamWriter>* input,
     scoped_ptr<ByteStreamReader>* output);
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_DOWNLOAD_BYTE_STREAM_H_
+