cline@cheetah.ece.clarkson.edu (Marshall Cline) (12/04/90)
Former Subject: Re: "foo operator+()" vs "foo& operator+()" (sorry; the new subject and the old wouldn't fit on the same line) In article <2753DDF4.7359@tct.uucp> chip@tct.uucp (Chip Salzenberg) writes: >According to Reid Ellis <rae@gpu.utcs.toronto.edu>: >> foo & foo::operator+=(const foo &f) { ... } >> foo & foo::operator+(const foo &f) >> { static foo sf; sf = *this; return sf += f; } >>The only way to get "caught" using this is if you hold >>the return value in a reference, as in: >> foo &rf = foo1 + foo2; >I believe that: > foo f = (foo1 + (foo2 + foo3)); >is likely to break using the reference-to-static trick. Even foo f=((foo1+foo2)+foo3) will break if operator=(const foo& f) isn't careful! Ie: if operator= doesn't check whether `this' points to `sf', BOOM! Nothing sophisticated in what follows; just a couple of tricky-to-find (and sadly common!) C++ errors. I've found it all too easy to write an assignment operator that doesn't check if it's assigning to itself (no, I'm *not* even hinting that this policy baggage ought to be added to the compiler!!!!!!!!). But, it's a subtle bug that is just *waiting* to bite you, and when (not if) it bites, it's rather nasty to track down. Ex: consider: | class String { | char* s; //the String data | int L; //cache the length | public: | String(const String& S) : s(new char[S.L+1]), L(S.L) { strcpy(s, S.s); } | String& operator=(const String& S) | { delete s; L = S.L; s = new char[L+1]; strcpy(s, S.s); } | //^^^^^^^^-- bug#1 | String(const char* S) : L(strlen(S)), s(new char[L+1]) { strcpy(s, S); } | //^^^-- bug#2 | String() : s(new char[1]), L(0) { s[0] = '\0'; } | //... | }; This class doesn't check the return value of `new' to make sure it's non-0, so these are other bugs (can't wait for exceptions!), however the two nasty ones are indicated. Bug#1: suppose you do end up with an s=s situation (which isn't that hard, considering you can have references passed around to other functions). The assignment operator dutifully delete's *both* its destination *and* source char*'s, since they're both the same thing! Then you do a strcpy from memory that is now owned by the system, and you *might* get the right result, or you might get gobbledegook (or a core dump or ...). Solution: | String& String::operator=(const String& S) | {if (this!=&that) {delete s; L = S.L; s = new char[L+1]; strcpy(s, S.s);}} Bug#2: just because L appears in the ``init-list'' before s does NOT mean L is initialized before S. Initialization occurs in the order subobjects appear in the class defn (there is a very good reason for this: destructors must be able to reliably destroy subobjects in the reverse order they were constructed, so all constructors must construct subobjects in the *same* order, and `appearance in the class' was selected as this `standard' order). Thus this ctor gets a garbage number of chars via `new', *then* it sets L to strlen. The result will depend on the phase of the moon. BTW: bug #2 can be eliminted in this case by using assignment rather than initialization in the ctor, such as: | String(const char* S) {L=strlen(S); s=new char[L+1]; strcpy(s,S);} and certainly it has no time or space cost since neither `char*' nor `int' have empty ctors, but I've found it useful to ALWAYS use an init list (well, *almost* always :-), even if the class' subobjects are all of builtin types. Obviously there are cases where you can't `initialize' a subobject so you *have* to `assign', but the possibility of a *huge* time cost is frightening. Ex: | class Person { | String name; | //...other subobjects... | public: | Person(const char* name_) { name = name_; } | }; | | main() | { | Person neighbor = "Joe"; | } Constructing `neighbor' involves the following: * Person::Person(const char*) gets called with parameter "Joe". * Person::Person(const char*) has no init list, so member object Person::name gets constructed by the default ctor String::String(). This allocates a 1 byte area from freestore for the '\0' (String's default ctor could've been smarter, such as using a static "" area for the zero-len String). * A temporary "Joe" String is created using String::String(const char*). This is another freestore allocation and a strcpy(). * String::operator=(const String&) is called with the temporary String. This will `delete' the old string in `s', use another `new' to get space for the new string, and do another strcpy(). * The temporary String gets destroyed, yet another freestore operation. Final score: 3 `new', 2 `strcpy()', and 2 `delete' Total: 7 `cost units' Compare this to an initialization-list version: Person::Person(const char* name_) : name(name_) { } Cost to initialize `neighbor': * Person::Person(const char*) gets called with parameter "Joe" * Person::Person(const char*) *has* an init list, so member object `name' is initialized from `name_' with String::String(const char*) Final score: 1 `new', 1 `strcpy()', 0 `delete' Total: 2 `cost units' Sorry if this was obvious to everyone. But with a 7.5 month doubling rate of new C++ users, there are bound to be lots and lots of people who are learning the language. So, I hope this helped *someone*! Marshall Cline -- PS: If your company is interested in on-site C++/OOD training, drop me a line! PPS: Career search in progress; ECE faculty; research oriented; will send vita. -- Marshall Cline / Asst.Prof / ECE Dept / Clarkson Univ / Potsdam, NY 13676 cline@sun.soe.clarkson.edu / Bitnet:BH0W@CLUTX / uunet!clutx.clarkson.edu!bh0w Voice: 315-268-3868 / Secretary: 315-268-6511 / FAX: 315-268-7600