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]> >> > >> >> > >> >> >> . >> >> >> 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. > > -- 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.
