Problem overloading extraction operator

[EM.Bateman]

Working on Visual Studio .Net I've implemented a class:

#ifndef CONTRIBUTOR_H
#define CONTRIBUTOR_H

enum Gender {male=1, female, unk};

#include <iostream>
#include <iomanip>
#include <string>
using namespace std;

class Contributor
{
friend ostream& operator << (ostream& output, Contributor& RHS);
friend istream& operator >> (istream& input, Contributor& RHS);

public:
Contributor();
Contributor(string Name, double Contribution, Gender MF, int IDKey);
Contributor(const Contributor &copy);
Contributor& operator=(const Contributor& RHS);
bool operator== (const Contributor& RHS);
~Contributor() {cout << "In Contributor Destructor" << endl;}

private:
std::string Name;
double Contribution;
Gender MF;
int IDKey;
};

Then I tried to overload the extraction operator thus:

istream& operator >> (istream& input, Contributor& RHS)
{
cout << "\nEnter a new contributor's name: ";
std::getline (cin, RHS.Name);
cout << "Enter the amount of their contribution: ";
cin >> RHS.Contribution;
cout << "What is the gender of the contributor? ";
cin >> RHS.MF;
cout << "Assign an ID Key to the contributor: ";
cin >> RHS.IDKey;

return input;
}

But I keep getting this error: error C2679: binary '>>' : no operator
found which takes a right-hand operand of type 'Gender' (or there is
no acceptable conversion)

I've got gender defined in my constructors as well as the overloading
implementation. I don't understand why I'm getting this error. Can
someone help?

Many thanks.

 

[red floyd]

Working on Visual Studio .Net I've implemented a class:

#ifndef CONTRIBUTOR_H
#define CONTRIBUTOR_H

enum Gender {male=1, female, unk};

#include <iostream>
#include <iomanip>
#include <string>
using namespace std;

Do *NOT* put using namespace std; into a header file.

class Contributor
{
friend ostream& operator << (ostream& output, Contributor& RHS);
friend istream& operator >> (istream& input, Contributor& RHS);

public:
Contributor();
Contributor(string Name, double Contribution, Gender MF, int IDKey);
Contributor(const Contributor &copy);
Contributor& operator=(const Contributor& RHS);
bool operator== (const Contributor& RHS);
~Contributor() {cout << "In Contributor Destructor" << endl;}

private:
std::string Name;
double Contribution;
Gender MF;
int IDKey;
};

Then I tried to overload the extraction operator thus:

istream& operator >> (istream& input, Contributor& RHS)
{
cout << "\nEnter a new contributor's name: ";
std::getline (cin, RHS.Name);
cout << "Enter the amount of their contribution: ";
cin >> RHS.Contribution;
cout << "What is the gender of the contributor? ";
cin >> RHS.MF; Your error is here.
cout << "Assign an ID Key to the contributor: ";
cin >> RHS.IDKey;

return input;
}

But I keep getting this error: error C2679: binary '>>' : no operator
found which takes a right-hand operand of type 'Gender' (or there is
no acceptable conversion)

I've got gender defined in my constructors as well as the overloading
implementation. I don't understand why I'm getting this error. Can
someone help?

Your compiler is telling you exactly what is wrong. As part of
operator>>(istream&,Contributor&), you are using the extraction operator
on an object of type Gender (specifically, RHS.MF). The extraction
operator is not defined for that enumeration.

Define it (and the insertion operator), and all will be well.

 

[Alf P. Steinbach]

* (e-mail address removed):

Working on Visual Studio .Net I've implemented a class:

#ifndef CONTRIBUTOR_H
#define CONTRIBUTOR_H

enum Gender {male=1, female, unk};

OK so far.

#include <iostream>
#include <iomanip>

Instead of these heavy-weight headers, consider using the light-weight

#include <string>
using namespace std;

Never have "using namespace std" in a header file.

class Contributor
{
friend ostream& operator << (ostream& output, Contributor& RHS);
friend istream& operator >> (istream& input, Contributor& RHS);

Reserve all uppercase for macros. See this group's FAQ as well as
Bjarne Stroustrup's FAQs.

public:
Contributor();
Contributor(string Name, double Contribution, Gender MF, int IDKey);

Unrelated to C++ but, what does it matter whether a Contributor has a
penis, a vagina, or perhaps nothing? I'd remove that. Unless the
sexual organs really matter for the contribution (whatever it is). If,
for example, the gender information is used to establish a proper
honorific in a computer generated thank-you letter, it would be much
better design-wise to store the proper honorific. It would also yield
simpler code; see below.

Contributor(const Contributor &copy);
Contributor& operator=(const Contributor& RHS);
bool operator== (const Contributor& RHS);
~Contributor() {cout << "In Contributor Destructor" << endl;}

private:
std::string Name;
double Contribution;
Gender MF;
int IDKey;
};

Note: with the data members you have there's no need to define a copy
constructor and assignment operator; the compiler generated ones will do
nicely.

Then I tried to overload the extraction operator thus:

istream& operator >> (istream& input, Contributor& RHS)
{
cout << "\nEnter a new contributor's name: ";
std::getline (cin, RHS.Name);
cout << "Enter the amount of their contribution: ";
cin >> RHS.Contribution;
cout << "What is the gender of the contributor? ";
cin >> RHS.MF;
cout << "Assign an ID Key to the contributor: ";
cin >> RHS.IDKey;

return input;
}

Consider how you would test your class with prepared data in a file.
Consider how you'd use it in a batch job (silent, no user interaction).

But I keep getting this error: error C2679: binary '>>' : no operator
found which takes a right-hand operand of type 'Gender' (or there is
no acceptable conversion)

I've got gender defined in my constructors as well as the overloading
implementation. I don't understand why I'm getting this error. Can
someone help?

Gender is defined as an enum type. You can provide an extractor for it.
That function (example below) /defines/, implicitly, the textual
representation of genders, here as simple decimal integers:

istream& operator>>( istream& input, Gender& g )
{
int genderVal;
input >> genderVal;
if( input.fail() ) { throw std::runtime_error( "Ouch ouch" ); }
// Here perhaps check the value and throw exception if not OK.
// Assuming it is OK:
g = static_cast<Gender>( genderVal );
return input;
}

Note that this would be very much easier it you stored a preferred
honorific (string) instead of a gender (enum).

Hth.,

- Alf

 

[Old Wolf]

* (e-mail address removed):

Unrelated to C++ but, what does it matter whether a Contributor has a
penis, a vagina, or perhaps nothing? I'd remove that.

I for one, would appreciate not seeing political correctness
ranting here. I get enough of it in the state-sponsored media.

istream& operator>>( istream& input, Gender& g )
{
int genderVal;
input >> genderVal;
if( input.fail() ) { throw std::runtime_error( "Ouch ouch" ); }
// Here perhaps check the value and throw exception if not OK.
// Assuming it is OK:
g = static_cast<Gender>( genderVal );
return input;
}

This code will allow values to be assigned to 'g' that are not valid
members of the enum.

 

[Alf P. Steinbach]

* Old Wolf:

I for one, would appreciate not seeing political correctness
ranting here. I get enough of it in the state-sponsored media.

The paragraph you've quoted the start of is simply about choosing a bad
option both designwise and codingwise, versus choosing something better:
no political correctness issue, and no ranting.

Please read before you write.

This code will allow values to be assigned to 'g' that are not valid
members of the enum.

Please read before you write.

 

[Old Wolf]

* Old Wolf:



The paragraph you've quoted the start of is simply about choosing a bad
option both designwise and codingwise, versus choosing something better:
no political correctness issue, and no ranting.

Please read before you write.

The issue is whether it matters what sex a contributor is.
This is nothing to do with coding. Code-design decisions
shouldn't be influenced by /minor/ coding considerations.

In fact this is not even an issue of code design. It is an
issue of what the program is supposed to actually do,
which should almost never be influenced by coding
considerations. Who are you to say that what is or is
not a relevant piece of information for this project?

You have no idea what the OP's situation is, or what he is
intending to do with this information. You hazard a guess
with no rationale and then criticise the OP's code for not
being 'better' and 'easier' than your guess (never mind what
those words are supposed to mean).

In case I need to spell it out: it is better to have more lines
of code to achieve the specified objective, than a short clear
program which does the wrong thing.

 

[Alf P. Steinbach]

* Old Wolf:

The issue is whether it matters what sex a contributor is.
This is nothing to do with coding. Code-design decisions
shouldn't be influenced by /minor/ coding considerations.

In fact this is not even an issue of code design. It is an
issue of what the program is supposed to actually do,
which should almost never be influenced by coding
considerations. Who are you to say that what is or is
not a relevant piece of information for this project?

You have no idea what the OP's situation is, or what he is
intending to do with this information. You hazard a guess
with no rationale and then criticise the OP's code for not
being 'better' and 'easier' than your guess (never mind what
those words are supposed to mean).

In case I need to spell it out: it is better to have more lines
of code to achieve the specified objective, than a short clear
program which does the wrong thing.

Well, again I can only ask you to please read before you write.

Please read before you write.

No absolute judgement about that issue was communicated by me. The very
next sentence in the paragraph you quoted a little bit of, stated the
conditions under which the advice would be valid. You're trolling.

 

[EM.Bateman]

I for one, would appreciate not seeing political correctness
ranting here. I get enough of it in the state-sponsored media.


This code will allow values to be assigned to 'g' that are not valid
members of the enum.

I'm not very skilled at C++. Should this be set as a separate
function in my .cpp file or should it go inside the current extraction
overloading code?

 

[EM.Bateman]

I'm not very skilled at C++. Should this be set as a separate
function in my .cpp file or should it go inside the current extraction
overloading code?- Hide quoted text -

- Show quoted text -

This is an exercise for my Data Structures class in college. I'm
marginal at C++ coding, at best. I didn't design the class or the
program, I just have to implement it. I guess I'm over my head.

 

[Alf P. Steinbach]

* (e-mail address removed):

I'm not very skilled at C++. Should this be set as a separate
function in my .cpp file or should it go inside the current extraction
overloading code?

A declaration of the function belongs with the enum. The most natural
given the code packaging you've already chosen is to place the
definition in the implementation file. You'd do well to replace the
comments about possible code, with actual code: either throwing an
exception, or communicating an input error via the stream error state.

 

[James Kanze]

* (e-mail address removed):

Gender is defined as an enum type. You can provide an extractor for it.
That function (example below) /defines/, implicitly, the textual
representation of genders, here as simple decimal integers:

istream& operator>>( istream& input, Gender& g )
{
int genderVal;
input >> genderVal;
if( input.fail() ) { throw std::runtime_error( "Ouch ouch" ); }

This violates the normal contract of operator>>, which is to
signal errors by means of the failbit (or the badbit, for hard
IO errors).

// Here perhaps check the value and throw exception if not OK.
// Assuming it is OK:
g = static_cast<Gender>( genderVal );

The range check is essential if you want to avoid undefined
behavior. It should set failbit, however, and not throw an
exception.

return input;
}

A more reasonable implementation would use a single character,
'M' or 'F', to choose (with maybe '-' for unknown). The same
principle applies, however: input into a variable (of type
char), then:

switch ( value ) {
case 'M' :
case 'm' :
g = male ;
break ;

case 'F' :
case 'f' :
g = female ;
break ;

case '-' :
g = unk ;
break ;

default :
input.setstate( std::ios::failbit ) ;
break ;
}

 

[James Kanze]

* Old Wolf:

The paragraph you've quoted the start of is simply about choosing a bad
option both designwise and codingwise, versus choosing something better:
no political correctness issue, and no ranting.

The problem is that the original poster didn't give any
information as to what the field might be used for. There was
no reason to suppose that it was used to generate the honorific
(which, as you point out, should be a separate string). A lot
of contexts still require maintaining such information, by law,
and others will do so for statistical purposes.

Please read before you write.

Please read before you write.

You mention the fact in comments, but you fail to point out that
doing so is in fact undefined behavior, which means that such a
test really isn't optional. (On the other hand, throwing an
exception in case of failure is NOT correct behavior for an
extractor.)

 

[James Kanze]

I'm not very skilled at C++. Should this be set as a separate
function in my .cpp file or should it go inside the current extraction
overloading code?

It depends. Given that you've defined Gender outside the class,
making it publicly available, I'd declare the function in a
header, and implement it in a source file somewhere. Depending
on the use of the class, however, it might be valid to define
the type in class, and implement the input function directly
inline.

 

[Alf P. Steinbach]

* James Kanze:

This violates the normal contract of operator>>, which is to
signal errors by means of the failbit (or the badbit, for hard
IO errors).

Not really. First, streams can be set to throw exceptions: you can
choose between two contracts, and the choice is yours. The only problem
with that is that the contract which isn't your "normal" one is just as
inherently flawed as is your "normal" one, and perhaps a little worse
(as is most everything with iostreams, except for trivial toy program
usage). Second, in any application there may be and quite likely is a
contractual regime above the level of direct calls to standard library
functions, where stream errors are converted to exceptions (in a far
more reliable way than streams can do themselves), and it makes just as
much sense to do that directly than to provide a higher level wrapper.

The range check is essential if you want to avoid undefined
behavior.

Not really. It all depends on what the function's contract is: what
it requires and what results it guarantees in return. I happen to agree
with you that it's best to have the check in place, not the least
because without it there's a likely entry vector for /badware/, but e.g.
Betrand Meyer has argued extremely forcefully the opposite viewpoint.

It should set failbit, however, and not throw an
exception.

There I definitely disagree with you (see above, plus, adding work to
conform to a harebrained scheme like that particular contract, of the
two possible ones, is IMHO just silly), but again, there are people --
I don't recollect names and am too lazy to Google -- who have argued
very forcefully your point of view here, which isn't difficult since the
natural assumption of readers is that as part of the standard library,
there must be something worthwhile in the design of iostreams.

A more reasonable implementation would use a single character,
'M' or 'F', to choose (with maybe '-' for unknown). The same
principle applies, however: input into a variable (of type
char), then:

switch ( value ) {
case 'M' :
case 'm' :
g = male ;
break ;

case 'F' :
case 'f' :
g = female ;
break ;

case '-' :
g = unk ;
break ;

default :
input.setstate( std::ios::failbit ) ;
break ;
}

Well yes, agreed, but my simple example was less to type. ;-)

 

[Alf P. Steinbach]

* James Kanze:

The problem is that the original poster didn't give any
information as to what the field might be used for. There was
no reason to suppose that it was used to generate the honorific
(which, as you point out, should be a separate string). A lot
of contexts still require maintaining such information, by law,
and others will do so for statistical purposes.

In private communication he's informed me it was a requirement of a
college assignment. Anyway, it's not a problem. The advice was
properly qualified. I'd have thrown in weasel words too if I'd known
people would get so riled up about it. It was good advice, but note
that the quoting snipped the most significant parts.

You mention the fact in comments, but you fail to point out that
doing so is in fact undefined behavior

It isn't undefined behavior, although it can be. As an analogy, x*y
isn't undefined behavior, although it can be, depending on what's known
about x and y. Adding checks everywhere there's a /possibility/ of UB
when you remove all knowledge about execution context, means adding
checks around every basic operation, and that's just silly.

, which means that such a
test really isn't optional.
No.


(On the other hand, throwing an
exception in case of failure is NOT correct behavior for an
extractor.)

It can be correct, it can be incorrect, it all depends on context, just
like the possible UB of multiplication. It's the simplest thing to do.
And it's far more reliable than setting badbit.

 

[Alf P. Steinbach]

* Alf P. Steinbach:

* James Kanze:

istream& operator>>( istream& input, Gender& g )
{
int genderVal;
input >> genderVal;
if( input.fail() ) { throw std::runtime_error( "Ouch ouch" ); }
// Here perhaps check the value and throw exception if not OK.
// Assuming it is OK:
g = static_cast<Gender>( genderVal );
return input;
}

[Old Wolf] This code will allow values to be assigned to 'g' that are not valid
members of the enum.

Please read before you write.

You mention the fact in comments, but you fail to point out that
doing so is in fact undefined behavior

It isn't undefined behavior, although it can be. As an analogy, x*y
isn't undefined behavior, although it can be, depending on what's known
about x and y. Adding checks everywhere there's a /possibility/ of UB
when you remove all knowledge about execution context, means adding
checks around every basic operation, and that's just silly.

Sorry, on second thoughts (1) disregarding the built-in UB of iostreams
here, not checking can't be UB (it can at worst be unspecified result,
§5.2.9/7, which means it depends on the implementation, §1.3.13) and (2)
the function is therefore potentially more useful without the check or
with the check optional (one could always define a checking wrapper).
Somehow I instinctively did The Right Thing, but your comment about UB
confused me when defending that. It's just that I'm used to you being
Always Right about basic facts, and so I didn't think.

 

[EM.Bateman]

* Alf P. Steinbach:

* James Kanze:

istream& operator>>( istream& input, Gender& g )
{
int genderVal;
input >> genderVal;
if( input.fail() ) { throw std::runtime_error( "Ouch ouch" ); }
// Here perhaps check the value and throw exception if not OK.
// Assuming it is OK:
g = static_cast<Gender>( genderVal );
return input;
}
[Old Wolf] This code will allow values to be assigned to 'g' that are not valid
members of the enum.
Please read before you write.
You mention the fact in comments, but you fail to point out that
doing so is in fact undefined behavior

It isn't undefined behavior, although it can be. As an analogy, x*y
isn't undefined behavior, although it can be, depending on what's known
about x and y. Adding checks everywhere there's a /possibility/ of UB
when you remove all knowledge about execution context, means adding
checks around every basic operation, and that's just silly.

Sorry, on second thoughts (1) disregarding the built-in UB of iostreams
here, not checking can't be UB (it can at worst be unspecified result,
§5.2.9/7, which means it depends on the implementation, §1.3.13) and (2)
the function is therefore potentially more useful without the check or
with the check optional (one could always define a checking wrapper).
Somehow I instinctively did The Right Thing, but your comment about UB
confused me when defending that. It's just that I'm used to you being
Always Right about basic facts, and so I didn't think.

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?- Hide quoted text -

- Show quoted text -

OK, you guys have just lost me now. I've got it working but I have no
clue whether it's right or not. Here I quote the requirements of the
lab:
Data members:
std::string Name; //the name of the contributor - use the
Standard Library string class as the type
double Contribution //the amount of the contribution given
to the cause
enum Gender {male, female, none}; //the gender of the
contributor
int IDKey; //identifier key for the Hashing list if desired
Member Functions/Methods:
default Constructor and overloaded Constructor accepting a
string for the name and the appropriate other information
Copy Constructor
Destructor
Overloaded << (insertion operator for your Contributor
objects) to show the object
Overloaded >> (extraction operator for your Contributor
objects) to load the object
Overloaded assignment (=)
Overloaded <, >, ==, != (return a C++ bool type - use the
contributor's name to determine order, use all member data to
determine equality or non-equality)

Going forward we will be using this class to build a linked list
(singularly and doubly-linked as well as circular), a stack, a queue
and a binary tree.

Does this help?