I agree, the more explicit the better. I'm all for adopting Google's policy on Copy constructors in its entirety.
- Doug On Mon, Apr 5, 2010 at 10:26 AM, conferno <[email protected]> wrote: > Doug, > > I think, it it better to have the constructors visible in the code. > It makes things clear. > > Nevermind, just a question of style :) > > 2010/4/5, Doug Judd <[email protected]>: > > This is what was intended. Are you suggesting that there is a more > > efficient way to do this (InetAddr -> CommAddress -> Comm::method) ? Or > are > > you just suggesting that implicit object construction is bad? > > > > - Doug > > > > On Sun, Apr 4, 2010 at 12:51 PM, conferno <[email protected]> wrote: > > > >> Doug, > >> > >> I mean, if you declare CommAddress constructor explicit, the code will > >> look like (and what it implicitly is): > >> > >> comm->connect(CommAddress(addr), ... > >> comm->close_socket(CommAddress(addr), ... > >> comm->send_request(CommAddress(addr), ... > >> comm->send_response(CommAddress(addr), ... > >> > >> It is not only passing by value, it is construction of temporary > objects. > >> > >> 2010/4/4, Doug Judd <[email protected]>: > >> > The passing of InetAddr by value is intentional. The InetAddr > structure > >> is > >> > so small that we (I) deemed it better from a performance standpoint to > >> pass > >> > by value instead of take the hit of an extra level of indirection > >> introduce > >> > with a reference. > >> > > >> > On Sun, Apr 4, 2010 at 11:47 AM, conferno <[email protected]> wrote: > >> > > >> >> Doug, > >> >> > >> >> if you take care of performance hit of object copying, the rule "Use > >> >> the C++ keyword explicit for constructors with one argument." would > be > >> >> your friend as well. > >> >> > >> >> There are a lot of places where objects are constructed implicitly. > >> >> For example, CommAddress out of InetAddr. > >> >> > >> >> 2010/4/4, Doug Judd <[email protected]>: > >> >> > Hi Conferno, > >> >> > > >> >> > I agree that this is probably a good rule that we should start > >> adopting. > >> >> I > >> >> > can think of another serious performance bug that was introduced in > >> the > >> >> past > >> >> > due to inadvertently making a copy of an object. I cringe at the > >> >> > thought > >> >> of > >> >> > sweeping through the entire code base and adding that macro to > every > >> >> class. > >> >> > Maybe we can start with some of the big ones (RangeServer, Range, > >> >> > AccessGroup, CellStore, etc.) and then slowly introduce it to other > >> >> classes > >> >> > over time. I just filed issue 434 ( > >> >> > http://code.google.com/p/hypertable/issues/detail?id=434) for this > >> one. > >> >> > Thanks. > >> >> > > >> >> > - Doug > >> >> > > >> >> > P.S. I don't particularly believe in the 80 column rule. With C++, > >> >> > 80 > >> >> > columns is inadequate. It may be a little more doable with C or > >> >> > other > >> >> terse > >> >> > languages, but with namespaces, nested namespaces, nested classes, > >> etc. > >> >> 80 > >> >> > columns is unreasonably small. Especially given Google's rule of > not > >> >> > allowing the 'using' keyword. I do try to keep lines under 80 > >> columns, > >> >> but > >> >> > if the only way to keep the line under 80 columns is to make it > look > >> >> really > >> >> > ugly, then I'll break the rule. > >> >> > > >> >> > On Fri, Apr 2, 2010 at 9:21 PM, conferno <[email protected]> > wrote: > >> >> > > >> >> >> Hi > >> >> >> > >> >> >> I have seen that you took something from <a > >> >> >> href=" > http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml > >> >> >> ">Google > >> >> >> C++ Style Guide</a>: > >> >> >> two-spaces indentation, 80 char line limit, no spaces inside > >> >> >> parentheses, define guards in headers, using namespaces, ... > >> >> >> > >> >> >> What do you thing to take more rules from that document ? > >> >> >> > >> >> >> They have right ideas, and also automatic checking tools. > >> >> >> > >> >> >> For example <a href=" > >> >> >> > >> >> > >> > http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=Copy_Constructors#Copy_Constructors > >> >> >> ">the > >> >> >> rule about copy constructors</a> could prevent issue 422. > >> >> >> > >> >> >> -- > >> >> >> You received this message because you are subscribed to the Google > >> >> Groups > >> >> >> "Hypertable Development" group. > >> >> >> To post to this group, send email to > [email protected] > >> . > >> >> >> To unsubscribe from this group, send email to > >> >> >> [email protected]<hypertable-dev%[email protected]> > <hypertable-dev%[email protected]<hypertable-dev%[email protected]> > > > >> <hypertable-dev%[email protected]<hypertable-dev%[email protected]> > <hypertable-dev%[email protected]<hypertable-dev%[email protected]> > > > >> > > >> >> <hypertable-dev%[email protected]<hypertable-dev%[email protected]> > <hypertable-dev%[email protected]<hypertable-dev%[email protected]> > > > >> <hypertable-dev%[email protected]<hypertable-dev%[email protected]> > <hypertable-dev%[email protected]<hypertable-dev%[email protected]> > > > >> > > >> >> > > >> >> >> . > >> >> >> For more options, visit this group at > >> >> >> http://groups.google.com/group/hypertable-dev?hl=en. > >> >> >> > >> >> >> > >> >> > > >> >> > -- > >> >> > You received this message because you are subscribed to the Google > >> >> > Groups > >> >> > "Hypertable Development" group. > >> >> > To post to this group, send email to > [email protected]. > >> >> > To unsubscribe from this group, send email to > >> >> > [email protected]<hypertable-dev%[email protected]> > <hypertable-dev%[email protected]<hypertable-dev%[email protected]> > > > >> <hypertable-dev%[email protected]<hypertable-dev%[email protected]> > <hypertable-dev%[email protected]<hypertable-dev%[email protected]> > > > >> > > >> >> . > >> >> > For more options, visit this group at > >> >> > http://groups.google.com/group/hypertable-dev?hl=en. > >> >> > > >> >> > > >> >> > >> >> -- > >> >> You received this message because you are subscribed to the Google > >> Groups > >> >> "Hypertable Development" group. > >> >> To post to this group, send email to [email protected] > . > >> >> To unsubscribe from this group, send email to > >> >> [email protected]<hypertable-dev%[email protected]> > <hypertable-dev%[email protected]<hypertable-dev%[email protected]> > > > >> <hypertable-dev%[email protected]<hypertable-dev%[email protected]> > <hypertable-dev%[email protected]<hypertable-dev%[email protected]> > > > >> > > >> >> . > >> >> For more options, visit this group at > >> >> http://groups.google.com/group/hypertable-dev?hl=en. > >> >> > >> >> > >> > > >> > -- > >> > You received this message because you are subscribed to the Google > >> > Groups > >> > "Hypertable Development" group. > >> > To post to this group, send email to [email protected]. > >> > To unsubscribe from this group, send email to > >> > [email protected]<hypertable-dev%[email protected]> > <hypertable-dev%[email protected]<hypertable-dev%[email protected]> > > > >> . > >> > For more options, visit this group at > >> > http://groups.google.com/group/hypertable-dev?hl=en. > >> > > >> > > >> > >> -- > >> You received this message because you are subscribed to the Google > Groups > >> "Hypertable Development" group. > >> To post to this group, send email to [email protected]. > >> To unsubscribe from this group, send email to > >> [email protected]<hypertable-dev%[email protected]> > <hypertable-dev%[email protected]<hypertable-dev%[email protected]> > > > >> . > >> For more options, visit this group at > >> http://groups.google.com/group/hypertable-dev?hl=en. > >> > >> > > > > -- > > You received this message because you are subscribed to the Google Groups > > "Hypertable Development" group. > > To post to this group, send email to [email protected]. > > To unsubscribe from this group, send email to > > [email protected]<hypertable-dev%[email protected]> > . > > For more options, visit this group at > > http://groups.google.com/group/hypertable-dev?hl=en. > > > > > > -- > You received this message because you are subscribed to the Google Groups > "Hypertable Development" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected]<hypertable-dev%[email protected]> > . > For more options, visit this group at > http://groups.google.com/group/hypertable-dev?hl=en. > > -- You received this message because you are subscribed to the Google Groups "Hypertable Development" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/hypertable-dev?hl=en.
