views:

352

answers:

5

I am trying to initialize an array of objects:

SinglyLinkedList offeredClasses[22] = {SinglyLinkedList("CSCE101"),SinglyLinkedList("CSCE101L"),SinglyLinkedList("CSCE150E"),SinglyLinkedList("CSCE150EL"),SinglyLinkedList("CSCE150EM"),SinglyLinkedList("CSCE150EML"),SinglyLinkedList("CSCE155"),SinglyLinkedList("CSCE155H"),SinglyLinkedList("CSCE156"),SinglyLinkedList("CSCE230"),SinglyLinkedList("CSCE230L"),SinglyLinkedList("CSCE235"),SinglyLinkedList("CSCE251"),SinglyLinkedList("CSCE310"),SinglyLinkedList("CSCE322"),SinglyLinkedList("CSCE361"),SinglyLinkedList("CSCE351"),SinglyLinkedList("CSCE451"),SinglyLinkedList("CSCE423"),SinglyLinkedList("CSCE428"),SinglyLinkedList("CSCE486"),SinglyLinkedList("CSCE487")};

but when I try to do this it keeps trying to call my copy constructor instead of an overloaded constructor. Any ideas to fix this?

the 2 constructors in question are:

SinglyLinkedList(string course); //Constructor
SinglyLinkedList(SinglyLinkedList & otherObj); //Copy Constructor

I need the copy constructor for other things so I can't remove it.

Thanks for your help!

A: 

What do you need the copy constructor for that ISN'T copy construction? If you need it for something else then you need to change that other thing. It makes for really hard to understand code, even if you ignore problems like you are currently suffering. This is the same problem that people cause in maths libraries by doing things like overloading operator^ to do a cross-product.

Use things for what they are meant to be used for and prevent later problems while making your code many times more maintainable.

Goz
That misses the point. The code above shouldn’t call the copy constructor *at all*.
Konrad Rudolph
Rubbish. MyClass x = MyClass( someInitialiser ) will call the copy constructor ...
Goz
Well, it doesn't necessarily call it - the optimisation to make it identical to `MyClass x(someInitialiser)` is permitted, although the copy constructor must be callable. This is occasionally useful to avoid the "most vexing parse", since the alternative is to keep adding parentheses until the expression is so bewildering to both you and the compiler that it gives up and treats it as declaring an object rather than a function.
Steve Jessop
relying on a compiler-specific optimisation to fix your problem is bad coding ...
Goz
No it isn't. It's my job to make my code clear and understandable, and the compiler's job to apply simple, common optimisations that are clearly documented in the C++ standard, to make it efficient. If it can't do that, I can live with slower code, or get a better compiler, or (last resort) hand-optimise code at the bottleneck. And it's not just me - the whole design of the STL relies on optional optimisations such as "inline" to produce reasonable code.
Steve Jessop
Wait a minute? You are saying that using a copy constructor for something OTHER than copy construction is writing "Clear and understandable" code? A copy constructor should be for copy construction. Using it for anything else introduces hard to find and solve bugs such as the OP is suffering from (Not to mention maintainability nightmares).
Goz
I don't think I'm saying that. Using a copy initialisation statement, knowing that a half-decent compiler can optimise it to direct initialisation, is writing clear and understandable code. I don't see that it uses the copy constructor for anything other than copy construction, all that's in question is whether or not copy construction actually occurs, or is optimised away. If any of my colleagues found this unclear, I would direct them (politely, I hope), to GOTW #1, which explains how intialisation expressions work in C++.
Steve Jessop
Also note that the questioner's "hard to find and solve bug" is just a missing const in the copy ctor. That could happen however it's used - in this case it's copy initialisation, but the same thing would happen if it were being used in a context where it is required to be called. I don't think the correct solution for the questioner missing this and not understanding the resulting error message (which on gcc is kind of vague), is for me to avoid copy elision. A C++ programmer is responsible for understanding elision: it's in the standard and it happens a lot (RVO being the other case).
Steve Jessop
+6  A: 

It appears theat your compiler is seriously broken. Your copy-constructor is declared with a non-const reference parameter. Such a copy-constructor cannot be invoked with temporary object as an argument, since a non-const reference cannot be bound to a temporary object.

Your initializers are temporary objects, meaning that there's absolutely no way a copy-constructor can be called here. If your compiler does it, it means that it is either broken or that you are using some set of settings that make it behave in a weird non-compliant way. Which compiler are you using?

That's the first part of the answer.

The second part is that brace-enclosed initializer lists are interpreted as copy-initialization in C++. In other words, the copy-constructor must be called in this case. There's no way around it (the call can be later optimized away, but the constructor must be available in any case). In this regard, your compiler is behaving "correctly", i.e. it makes an attempt to call the copy-constructor, as it should. Except that, as I said above, in your case it should issue an error (since the copy-constructor is not callable) instead of quetly calling it.

And, finally, the third part of the answer.

You are saying that the copy-constructor is called instead of the conversion constructor. In reality, both are called. If you look carefully, you'll see it. Firstly, the conversion constructor is called in order to create an intermediate temporary object of 'SinglyLinkedList' type from the string you supplied (that involves constructing a temporary 'std::string' object as well), and then the copy-constructor is called in order to initialize the array element from the temporary (this happens for each element in the array). This is how it should be in C++, assuming your copy-constrcutor is declared properly, i.e. with a const reference parameter. But with non-const reference parameter, the copy-constructor is not callable and the code is ill-formed.

AndreyT
This is something I didn't know. However, at least GCC appears to be perfectly capable of eliding those copy constructors. C++ allows eliding constructor calls, but the suitable copy constructor must be available nevertheless, even if it is not going to be used. It is possible that OP is getting a compiler error, and then the fix is to make the copy constructor take a const reference as it should.
UncleBens
You are right, the actual copy-constructor call for the temporary can be omitted. This is explicitly permitted by the standard. However, conceptually the call is there and the corresponding copy-constructor must be available. This a requirement and if it is not met, the program is ill-formed.
AndreyT
To be fair, OP said it's "trying" to call the copy ctor, not that it's succeeding. Even odds the compiler is not seriously broken. Instead he's getting an error saying that the required copy ctor isn't there, and interpreting this to mean it wants the copy ctor *instead* of the string ctor, when it fact it (quite rightly) wants it *as well*.
Steve Jessop
OK, that's wasn't clear to me from the original question. I also remember that some older versions if MS C++ compiler didn't refuse to bind references to temporary objects (which is non-compliant). The latest versions can do that as well, given the right set of compiler settings.
AndreyT
Oh, you may well be right, and I agree it's not clear in the question. It's just that when I see "trying to call", that's at least as likely to mean "trying and failing" rather than "trying and succeeding". When I say "even odds", I actually mean "could be either, I really don't know" rather than literally a 50-50 chance :-)
Steve Jessop
A: 

The signature of the construcors you are calling is SinglyLinkedList( char const * )

Which you have not provided So either produce a constructotr taking char const* or call them as SinglyLinkedList(string( "CSCE101" ))

There is no implicit conversion from char* to std::string - so your the compiler has to see which overload matches and it finds matching SinglyLinkedList.

Mark
Actually it is looking for `const char *` - there is a subtle difference. `char const *` is a pointer (that can't be changed) to `char` data (that can be changed). `const char *` is a pointer (that can be changed, but it doesn't matter because it's passed by value) to `char` data (that can't be changed)
Chris Lutz
@Chris: `const char *` and `char const *` are identical. Possibly you're thinking of `char * const`.
Steve Jessop
I write my C code by putting the const after the bit it refers to char const* is the same as const char* as onebyone says
Mark
+1  A: 

When you have single argument constructors always declare them explicit, e.g.

explicit SinglyLinkedList(string course); //Constructor
explicit SinglyLinkedList(SinglyLinkedList & otherObj); //Copy Constructor

That way you have better chance of calling the right constructor.

On the other hand, the constructor compiler is searching for is SinglyLinkedList( const char* ). Try to create string instance directly, e.g SinglyLinkedList( string("CSCE101") )

Alexander Pogrebnyak
It won't help, meaning that it won't change anything. The compiler is already calling the overloaded constructor. The original poster simply failed to notice that. However, regardless of what you do, the final stage of initialization will involve copy-constructor (copy from the temporary object to the actual array element). This is a requirement in C++. There's no way around it.
AndreyT
A: 

You might want to try changing your constructor declarations to:

explicit SinglyLinkedList(std::string const& name);
SinglyLinkedList(SinglyLinkedList const& other);

Then add in the assignment operator and destructor:

~SinglyLinkedList();
SinglyLinkedList& operator=(SinglyLinkedList const& other);

I'm surprised that it is compiling at all since without the consts the two constructors should be ambiguous.

D.Shawley
Won't help as the parameter passed in is a char* ont a string
Mark
D.Shawley