" *first = new string(*rhs.first);" cannot compile in MSVC

[fl]

Hi,
I am learning C++ with a web download small C++ routine. The member
function below the dot line cannot compile in MSVC express 2010. The
error message is:

1>c:\users\jeff\documents\visual studio 2010\projects\ldpc\person
\person.cpp(27): error C2679: binary '=' : no operator found which
takes a right-hand operand of type 'std::string *' (or there is no
acceptable conversion)

Because the web site is a university C++ course, I guess it should not
be a totally wrong. I also feel that line 27 usage is really strange.

What is your opinion? Thanks a lot.





....................

const Person& Person:perator=(const Person& rhs){
if( this != &rhs ){
delete first;
delete last;
(line 27): *first = new string(*rhs.first);
*last = new string(*rhs.last);
id = rhs.id;
}

return *this;
}

 

[Ian Collins]

Hi,
I am learning C++ with a web download small C++ routine. The member
function below the dot line cannot compile in MSVC express 2010. The
error message is:

1>c:\users\jeff\documents\visual studio 2010\projects\ldpc\person
\person.cpp(27): error C2679: binary '=' : no operator found which
takes a right-hand operand of type 'std::string *' (or there is no
acceptable conversion)

Because the web site is a university C++ course, I guess it should not
be a totally wrong. I also feel that line 27 usage is really strange.

What is your opinion? Thanks a lot.

....................

const Person& Person:perator=(const Person& rhs){
if( this !=&rhs ){
delete first;
delete last;
(line 27): *first = new string(*rhs.first);
*last = new string(*rhs.last);
id = rhs.id;
}

return *this;
}

The definition of Person would help (what are first and last?).

 

[fl]

The definition of Person would help (what are first and last?).

Thank you very much. Here is the Person definition file:

//http://www.cs.middlebury.edu/~dkauchak/classes/s10/cs062-s10/
problems/destructor
...........
#ifndef PERSON_H
#define PERSON_H

#include <string>

using namespace std;

class Person{
public:
Person(const string& first, const string& last, int id);
Person:erson(const Person& rhs);
const Person& Person:perator=(const Person& rhs);
string getFirst() const;
string getLast() const;
void setLast(const string& newLast);
int getId() const;
string toString() const;

private:
string* first;
string* last;
int id;
};

#endif

 

[Alf P. Steinbach]

Thank you very much. Here is the Person definition file:

//http://www.cs.middlebury.edu/~dkauchak/classes/s10/cs062-s10/
problems/destructor
..........
#ifndef PERSON_H
#define PERSON_H

#include<string>

using namespace std;

class Person{
public:
Person(const string& first, const string& last, int id);
Person:erson(const Person& rhs);
const Person& Person:perator=(const Person& rhs);
string getFirst() const;
string getLast() const;
void setLast(const string& newLast);
int getId() const;
string toString() const;

private:
string* first;
string* last;
int id;
};

The code is not just technically wrong, but also totally wrong-headed.

So the question is, is it meant as an example of incorrect and
wrongheaded code, or is it meant to be OK?

Anyway, first, never place

using namespace std;

in the global namespace in a header. That's a novice error.

Then, iinstead of e.g.

string* first;

do

std::string first;

and fix all the code that refers to that.

Cheers & hth.,

- Alf

 

[Ian Collins]

Thank you very much. Here is the Person definition file:

//http://www.cs.middlebury.edu/~dkauchak/classes/s10/cs062-s10/
problems/destructor
...........
#ifndef PERSON_H
#define PERSON_H

#include<string>

using namespace std;

Don't ever put this in a header!

class Person{
public:
Person(const string& first, const string& last, int id);
Person:erson(const Person& rhs);
const Person& Person:perator=(const Person& rhs);
string getFirst() const;
string getLast() const;
void setLast(const string& newLast);
int getId() const;
string toString() const;

private:
string* first;
string* last;
int id;
};

#endif

So your original code is attempting to assign a std::string* to a
std::string.


As Alf suggests, don't use pointers.

 

[BCFD36]

On 01/12/12 04:29 PM, fl wrote:

[stuff deleted

Don't ever put this in a header!

It would be very helpful to the self stated rookie if you stated WHY
this should not be done

As Alf suggests, don't use pointers.

See above comment. If you have been doing this awhile, the answer is
obvious. If you are an FNG, maybe not so obvious.

 

[Goran]

Hi,
I am learning C++ with a web download small C++ routine. The member
function below the dot line cannot compile in MSVC express 2010. The
error message is:

1>c:\users\jeff\documents\visual studio 2010\projects\ldpc\person
\person.cpp(27): error C2679: binary '=' : no operator found which
takes a right-hand operand of type 'std::string *' (or there is no
acceptable conversion)

Because the web site is a university C++ course, I guess it should not
be a totally wrong. I also feel that line 27 usage is really strange.

Looking at later given Person definition, it becomes clear that the
code indeed is totally wrong. In fact, it is staggeringly wrong. Not
only that, it's clear that the code has never been tried, as it can't
possibly compile.

Some of things that are wrong with it:

0. it can't compile. *first is a std::string, and new string(whatever)
is a std::string*. WTF were these people thinking!?
1. there's no good reason to use string* for first, last members. In
C, one would have used char*, true, but in C++, one would not have
used string*. string class' "raison d’être" is to avoid using pointers
and meddling manually with dynamic storage. So whoever wrote this
shows a staggering lack of understanding of both C and C++. As said
elsewhere, a plain string should have been used.
2. operator= is exception-unsafe. (EXTREMELY important programming
knowledge found e.g. here: http://en.wikipedia.org/wiki/Exception_handling#Exception_safety).
"delete first" (or last) in the beginning leave the object in
undefined state: if exception is thrown by e.g. a new later on, first
and/or last point to a deleted object, and that invalidates Person
object for any later use. Correct thing to do (presuming "string*
first/last") is e.g (part of the function inside "if"):

// "can throw" zone here
auto_ptr<string> f(new string(rhs.first));
auto_ptr<string> l(new string(rhs.last));
// "can't throw" zone here
delete first;
delete last;
first = f.release();
last = l.release();

Finally, if first/last are a plain string (not string*), then this
becomes e.g.

// "can throw" zone here
string f(rhs.first);
string l(rhs.l);
// "can't throw" zone here
first.swap(f);
last.swap(l);

(Clarification: both snippets presume that Person:perator= needs so-
called "strong exception safety, which is pretty usual for operator=,
hence they make a copy of data in a throwing zone, then assign in a
non-throwing one).

Goran.

 

[Juha Nieminen]

On 01/12/12 04:29 PM, fl wrote:

[stuff deleted

Don't ever put this in a header!

It would be very helpful to the self stated rookie if you stated WHY
this should not be done

One interesting question is where do newbies learn the magic line
"using namespace std;" from? Obviously they do not come up with it on
their own. They have to be *taught* it. Not only do they have to see it,
it must be so prominently used that they will start using it as a matter
of fact, without even thinking about it.

Who teaches newbies to use that dreadful line? Mostly pros who should
know better! They write books, they write online tutorials, they write
example code, all of which is used by newbies to learn the language.
Why are they doing this? They know better, yet they still do it.

The reason why that line should be avoided should become clear when
one thinks about it a bit: What is the purpose of namespaces? Why are
all the standard functions and types inside a namespace? (If it still
doesn't ring a bell, then: What happens when two types or functions have
the same name and parameters?)

In fact, "using namespace std;" and then avoiding the "std::" prefix
actually makes the code less readable. No longer can you spot usages of
the standard libraries with a quick visual glance and, more importantly,
usages of the more obscure standard library names becomes confusing
(because there's no visual way of distinguishing where that name is
coming from).

 

[Alf P. Steinbach]

On 01/12/12 04:29 PM, fl wrote:

[stuff deleted

#define PERSON_H

#include<string>

using namespace std;

Don't ever put this in a header!

It would be very helpful to the self stated rookie if you stated WHY
this should not be done

One interesting question is where do newbies learn the magic line
"using namespace std;" from? Obviously they do not come up with it on
their own. They have to be *taught* it. Not only do they have to see it,
it must be so prominently used that they will start using it as a matter
of fact, without even thinking about it.

Who teaches newbies to use that dreadful line? Mostly pros who should
know better! They write books, they write online tutorials, they write
example code, all of which is used by newbies to learn the language.
Why are they doing this? They know better, yet they still do it.

The reason why that line should be avoided should become clear when
one thinks about it a bit: What is the purpose of namespaces? Why are
all the standard functions and types inside a namespace? (If it still
doesn't ring a bell, then: What happens when two types or functions have
the same name and parameters?)

In fact, "using namespace std;" and then avoiding the "std::" prefix
actually makes the code less readable. No longer can you spot usages of
the standard libraries with a quick visual glance and, more importantly,
usages of the more obscure standard library names becomes confusing
(because there's no visual way of distinguishing where that name is
coming from).

I have to disagree a bit, even though I agree with the main sentiment
about name collisions.

The C++ standard library has many names that can possibly conflict with
user code. Like `std::distance`. I think that that particular one only
bit me once, but there are plenty more.

But then, OTOH., adopting mechanical rules (and disregarding the notion
of possible intelligence on the part of the coder) in order to easily
see whether the code is using some less well known standard library
function or type, well I think that that is adding a lot of work and
pain for no real gain. Those double colons are not just more to write,
and they're not just noisy. They also cause a certain much used brain
dead editor to foul up the indentation of the currently edited text.

Anyway, I'd rather see `string` than `std::string`. Except for one
place, namely in the global namespace in a header. And that's because
when it is in the global namespace in a header, it can cause name
collisions with other code.

I guess, summing up, that I lean towards intelligence and understanding
rather than just following mechanical rules; that one should ideally
know *why* and not rely on rules designed to statistically accomplish
most of the why when followed mechanically. But when a novice is
struggling with something, simple "do like this" and "don't do that"
advice (to be followed mechanically, without understanding) can get the
person on track fast, and in a much better position to learn. So there's
a place for everything, including mechanical rule-following.

Cheers,

- Alf

 

[Alf P. Steinbach]

The asymmetric nature of using 'std::' in
header files but not in .cpp files is bogus even if name collisions are
more likely in header files.

I think most people would agree with that, since (1) there is no general
advice to not use `std::` in .cpp files, and (2) although some people
who haven't thought about it advocate never using `using namespace std;`
in header files, competent people advocate something that's more
meaningful, namely to not use that at the global namespace level in a
header file.

In short, you're arguing against viewpoints that no-one has advocated.

One possible reason for such gross a misunderstanding could be that
those whose viewpoints you (inadvertently) misrepresented, possibly
including me, have not managed to explain things clearly enough for you?


Cheers & hth.,

- Alf

 

[Juha Nieminen]

"Importing the entire std namespace into the global scope brings in a
lot more symbols than vector and list; it also brings in standard
library implementational details. C++ headers are allowed to include
other C++ headers, so there is no way of knowing just what symbols got
pulled into global scope. It's best not to use 'using namespace std;'."

To be fair, though, internal implementation details of standard header
files (such as for example variables used inside template functions)
should always names starting with underscores, as the standard explicitly
reserves those names for the compiler, and using them in your own code
is not recommended (and that's the reason why!)

Some compilers, especially older ones, sometimes used internal names
not starting with underscores in their standard header files, which
sometimes caused problems.