std::map Object wirthin std::map?

[MikeCopeland]

I am trying to implement a std::map object with a structure that
contains a std::map object within each object (is that possible?). In
the class below I have a number of variables and functions that all seem
to work...except the std::map named "eyMap". The function "setYearMax"
fails when I execute the code, but other setters and getters work as
desired.
Is there something fundamentally wrong with this logic, or is here
another way to declare and use a map-like structure within this class?
Please advise. TIA

class DBE_Info
{
public:
void setYoB(int year);
void setAge(int age);
void setType(char eType);
void setYearMax(int eYear);
void setRefName(string name);
void updPart(void);
void normalizeAgeType(void);
void computeYoB(void);
int getYoB(void);
int getAge(void);
int getEYear(void);
string getRefName(void);
char getType(void);
int getPart(void);
int getYearMax(int eYear);
DBE_Info(); // This is the constructor
~DBE_Info(); // This is the destructor
private:
int dAge, eYear;
int birthYear;
int yearsPart;
char entType;
string refName;
map<int, int> eyMap;
map<int, int>::iterator eyIter;
};
// Member functions definitions including constructor
DBE_Info::DBE_Info(void)
{
birthYear = yearsPart = dAge = eYear = 0;
entType = 'A';
refName = "";
eyMap.clear(); // is this needed within the constructor?
}
DBE_Info::~DBE_Info(void) { return; }
void DBE_Info::setYoB(int year) { birthYear = year; return; }
void DBE_Info::setRefName(string name) { refName = name; return; }
string DBE_Info::getRefName(void) { return refName; }
int DBE_Info::getEYear(void) { return eYear; }
int DBE_Info::getYoB(void) { return birthYear; }
void DBE_Info::setType(char eType) { entType = (eType == ' ') ?
'A' : eType; return; }
char DBE_Info::getType(void) { return entType; }
void DBE_Info::updPart(void) { yearsPart++; return; }
int DBE_Info::getPart(void) { return yearsPart; }
void DBE_Info::computeYoB(void)
{
dAge = nAge, eYear = eventYear;
if(nAge >= maxAgegroup)
{
int qq = (cGender == 'M') ? 0 : 1;
entType = age2Str[cRCode-'a'][nAge][qq].cEntType;
return;
} // if
if(birthYear > 0) return;
int wYear = eventYear;
if((wYear >= 0) && (wYear < 100)) wYear +=1900;
if(wYear < EPOCH) wYear +=100;
birthYear = eventYear-nAge;
return;
} // computeYoB
void DBE_Info::setYearMax(int eYear)
{
eyMap[eYear] = maxAgegroup; // This FAILS!!
return;
} // setYearMax
int DBE_Info::getYearMax(int eYear)
{
eyIter = eyMap.find(eYear);
if(eyIter != eyMap.end()) return eyIter->second;
else
{
return 89;
}
} // getYearMax



---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

[Victor Bazarov]

On 8/2/2016 3:27 PM, MikeCopeland wrote:
> I am trying to implement a std::map object with a structure that
> contains a std::map object within each object (is that possible?).

Why wouldn't it be? You can have a vector of deques of sets of
strings... No particular limitation on what is contained in what.

> In
> the class below I have a number of variables and functions that all seem
> to work...except the std::map named "eyMap". The function "setYearMax"
> fails when I execute the code,

Should we now commence guessing how it "fails", or will you give us some
hint

> but other setters and getters work as
> desired.
> Is there something fundamentally wrong with this logic, or is here
> another way to declare and use a map-like structure within this class?

At a first glance there is nothing inherently wrong with it. Try FAQ
5.8 (if you can find it, look for mirrors of C++ FAQ Lite).

Don't use this C-ism. If you intend to have nothing between the
parentheses, don't put anything there. Ought to be

DBE_Info::DBE_Info()

> {
> birthYear = yearsPart = dAge = eYear = 0;
> entType = 'A';
> refName = "";

Don't use assignments. Use initializer list.

> eyMap.clear(); // is this needed within the constructor?

Nope.

> }
> DBE_Info::~DBE_Info(void) { return; }

Same note about "void" as above.

> void DBE_Info::setYoB(int year) { birthYear = year; return; }
> void DBE_Info::setRefName(string name) { refName = name; return; }

Don't pass objects by value. Pass them by const ref.

Try formatting your code better and add comments explaining your
intentions. Your code is not easy to read.

V
--
I do not respond to top-posted replies, please don't ask

[Paavo Helde]

On 2.08.2016 22:27, MikeCopeland wrote:
> I am trying to implement a std::map object with a structure that
> contains a std::map object within each object (is that possible?).

Yes, that's ok so far as you don't attempt to store a map object inside
itself.

That's OK.

> map<int, int>::iterator eyIter;

Storing an iterator long-term in the class seems not so good idea, why
do you think you need this?

> };
> // Member functions definitions including constructor
> DBE_Info::DBE_Info(void)
> {
> birthYear = yearsPart = dAge = eYear = 0;
> entType = 'A';
> refName = "";
> eyMap.clear(); // is this needed within the constructor?

No.

> }
> DBE_Info::~DBE_Info(void) { return; }
> void DBE_Info::setYoB(int year) { birthYear = year; return; }
> void DBE_Info::setRefName(string name) { refName = name; return; }
> string DBE_Info::getRefName(void) { return refName; }
> int DBE_Info::getEYear(void) { return eYear; }
> int DBE_Info::getYoB(void) { return birthYear; }
> void DBE_Info::setType(char eType) { entType = (eType == ' ') ?
> 'A' : eType; return; }
> char DBE_Info::getType(void) { return entType; }
> void DBE_Info::updPart(void) { yearsPart++; return; }
> int DBE_Info::getPart(void) { return yearsPart; }
> void DBE_Info::computeYoB(void)
> {
> dAge = nAge, eYear = eventYear;

Does not compile.

> if(nAge >= maxAgegroup)

Does not compile: no nAge, no maxAgegroup, etc.

> {
> int qq = (cGender == 'M') ? 0 : 1;
> entType = age2Str[cRCode-'a'][nAge][qq].cEntType;
> return;
> } // if
> if(birthYear > 0) return;
> int wYear = eventYear;
> if((wYear >= 0) && (wYear < 100)) wYear +=1900;
> if(wYear < EPOCH) wYear +=100;
> birthYear = eventYear-nAge;
> return;
> } // computeYoB
> void DBE_Info::setYearMax(int eYear)
> {
> eyMap[eYear] = maxAgegroup; // This FAILS!!

In principle something like this should work fine. This exact code
however fails already at compile time because there is no maxAgegroup
anywhere.

Maybe maxAgegroup should be a parameter here? Are you aware that a
function can have multiple parameters?

> return;
> } // setYearMax
> int DBE_Info::getYearMax(int eYear)
> {
> eyIter = eyMap.find(eYear);
> if(eyIter != eyMap.end()) return eyIter->second;
> else
> {
> return 89;
> }
> } // getYearMax
>

Your code contains a lot of noise (unneeded 'void' and 'return;'
thingies). Also it produces a lot of compile errors, which kind of
contradicts your claim it failed at run time. Anyway, you did not dare
to tell what error did you got.

Try to prepare a complete (preferably compilable) example of your code,
together with the main() function, demonstrating the problem. See
"http://www.graphics.cornell.edu/~martin/docs/c++-faq/how-to-post.html#[5.7]"

[MikeCopeland]

> Should we now commence guessing how it "fails", or will you give us some
> hint

It doesn't update the map object when I walk through the code with the
debugger, and there's nothing in the map object when I access the object
in later code. In essence, nothing happens in the code that fails.

> > DBE_Info::DBE_Info(void)

> Don't use this C-ism. If you intend to have nothing between the
> parentheses, don't put anything there. Ought to be

> DBE_Info::DBE_Info()

Point taken. Thanks for that, as well as the oher suggestions.

> Try formatting your code better and add comments explaining your
> intentions. Your code is not easy to read.

I culled my (numerous) comments in favor of posting even more code (that
would be more difficult to read). My apologies...

[MikeCopeland]

In article <0LOdnU2jDoMdZD3K...@giganews.com>,
myfir...@osa.pri.ee says...

>
> > I am trying to implement a std::map object with a structure that
> > contains a std::map object within each object (is that possible?).
>
> Yes, that's ok so far as you don't attempt to store a map object inside
> itself.
>

> That's OK.
>
> > map<int, int>::iterator eyIter;
>
> Storing an iterator long-term in the class seems not so good idea, why
> do you think you need this?

No reason, other than documenting how the map object is being
accessed. Using a global (within a loop) would be fine for me.

> Does not compile.

> > if(nAge >= maxAgegroup)

> Does not compile: no nAge, no maxAgegroup, etc.

True - because it's a global data item, as are some other variables
referenced in the code I posted. (Yes, I know the use of global
variables isn't recommended, but it's my bad...)

> Maybe maxAgegroup should be a parameter here? Are you aware that a
> function can have multiple parameters?

Yes. My laziness being exposed... 8<{{

> Your code contains a lot of noise (unneeded 'void' and 'return;'
> thingies). Also it produces a lot of compile errors, which kind of
> contradicts your claim it failed at run time. Anyway, you did not dare
> to tell what error did you got.

All true. I knew I was posting a lot here, but I didn't want to show
code that (I thought) wasn't pertinent to the issue. My apoligies.

[Mr Flibble]

On 02/08/2016 20:40, Victor Bazarov wrote:
> On 8/2/2016 3:27 PM, MikeCopeland wrote:
>> void DBE_Info::setRefName(string name) { refName = name; return; }
>
> Don't pass objects by value. Pass them by const ref.

And miss out on move semantics?

/Flibble

[Martin Shobe]

On 8/2/2016 3:45 PM, MikeCopeland wrote:
> In article <0LOdnU2jDoMdZD3K...@giganews.com>,
> myfir...@osa.pri.ee says...

>> Your code contains a lot of noise (unneeded 'void' and 'return;'
>> thingies). Also it produces a lot of compile errors, which kind of
>> contradicts your claim it failed at run time. Anyway, you did not dare
>> to tell what error did you got.
>
> All true. I knew I was posting a lot here, but I didn't want to show
> code that (I thought) wasn't pertinent to the issue. My apoligies.
>

Here's a suggestion when asking for help in the future. If you think
there's code that is not pertinent to the issue, remove it and see if
the issue goes away. If it does, you'll know you were wrong about it
being pertinent and you'll have a clue about what is actually wrong. If
it doesn't go away, you'll have a simpler example of the problem that
will compile.

Martin Shobe

[Barry Schwarz]

On Tue, 2 Aug 2016 12:27:58 -0700, MikeCopeland <mrc...@cox.net>
wrote:

> I am trying to implement a std::map object with a structure that
>contains a std::map object within each object (is that possible?). In
>the class below I have a number of variables and functions that all seem
>to work...except the std::map named "eyMap". The function "setYearMax"
>fails when I execute the code, but other setters and getters work as
>desired.

Since the code you posted never calls setYearMax, how are we supposed
to help.

<snip>

>void DBE_Info::setYearMax(int eYear)
>{
> eyMap[eYear] = maxAgegroup; // This FAILS!!

If we sign a non-disclosure agreement, will you tell us how it fails.
It would also be nice if you showed us the definition of maxAgegroup.

As an aid in debugging, you may want to print the value of
maxAgegroup, either in this function or just before calling it.

> return;
>} // setYearMax

--
Remove del for email

[Paavo Helde]

On 2.08.2016 23:45, MikeCopeland wrote:
>> Should we now commence guessing how it "fails", or will you give us some
>> hint
>
> It doesn't update the map object when I walk through the code with the
> debugger, and there's nothing in the map object when I access the object
> in later code. In essence, nothing happens in the code that fails.

This probably means that you are updating a wrong object, maybe a copy
of it which was made inadvertently somewhere along the path. By default
C++ uses value-and-copy semantics (unlike some other popular languages),
so one needs to use explicit reference syntax in places where copying is
to be avoided.

If you want more help you need to post the code which is actually
calling setYearMax().

[Paavo Helde]

On 2.08.2016 23:45, MikeCopeland wrote:

> In article <0LOdnU2jDoMdZD3K...@giganews.com>,
> myfir...@osa.pri.ee says...
>>
>>> I am trying to implement a std::map object with a structure that
>>> contains a std::map object within each object (is that possible?).
>>
>> Yes, that's ok so far as you don't attempt to store a map object inside
>> itself.
>>
>> That's OK.
>>
>>> map<int, int>::iterator eyIter;
>>
>> Storing an iterator long-term in the class seems not so good idea, why
>> do you think you need this?
>
> No reason, other than documenting how the map object is being
> accessed. Using a global (within a loop) would be fine for me.
>
>> Does not compile.
>
>>> if(nAge >= maxAgegroup)
>
>> Does not compile: no nAge, no maxAgegroup, etc.
>
> True - because it's a global data item, as are some other variables
> referenced in the code I posted. (Yes, I know the use of global
> variables isn't recommended, but it's my bad...)
>
>> Maybe maxAgegroup should be a parameter here? Are you aware that a
>> function can have multiple parameters?
>
> Yes. My laziness being exposed... 8<{{

We have the saying that the diligent (is this the right word?)
scratches, but the lazy washes. Same here, messing around with globals
keeps you busy all the time ;-)

[Juha Nieminen]

MikeCopeland <mrc...@cox.net> wrote:
> map<int, int>::iterator eyIter;

Even without reading the rest of the code, this line raises immediately
an alarm, especially since your class has no user-defined copy
constructor nor assignment operator.

If you store such an iterator as a member variable for longer than
the duration of a member function call, you are asking for trouble.
(And, if you only need the iterator for the duration of one member
function call, then there's seldom a need to store it as a member
variable. You can simply keep it locally, and if necessary pass it
around to other functions as a function parameter, which not only
makes it safer, but is also better design.)

If you really, really need to keep an iterator as a member variable,
you really have to implement a copy constructor and assignment
operator to take care of it when objects of this class are copied
around.

--- news://freenews.netfront.net/ - complaints: ne...@netfront.net ---

[Öö Tiib]

On Friday, 5 August 2016 17:57:04 UTC+3, Juha Nieminen wrote:
> MikeCopeland <mrc...@cox.net> wrote:
> > map<int, int>::iterator eyIter;
>
> Even without reading the rest of the code, this line raises immediately
> an alarm, especially since your class has no user-defined copy
> constructor nor assignment operator.

Yes 'eyMap' gets copied but iterator remains to map from what it was
copied in compiler-generated copy.
May be he needed it for debugging usage of that 'eyMap' member (with what
he indicated having issues)?

>
> If you store such an iterator as a member variable for longer than
> the duration of a member function call, you are asking for trouble.
> (And, if you only need the iterator for the duration of one member
> function call, then there's seldom a need to store it as a member
> variable. You can simply keep it locally, and if necessary pass it
> around to other functions as a function parameter, which not only
> makes it safer, but is also better design.)

It is not so bad as you put it with iterators (and pointers and
references) to elements of 'map' (or to 'set' or to 'list') since the
situations where those become invalidated are quite limited.
The map to what the iterator is is also owned by object of 'DBE_Info'
so I do not see harm of keeping some "bookmark" iterator to it.

>
> If you really, really need to keep an iterator as a member variable,
> you really have to implement a copy constructor and assignment
> operator to take care of it when objects of this class are copied
> around.

And also he should not to leave it uninitialized in constructor since
iterators do not have useful default values.

[Juha Nieminen]

Öö Tiib <oot...@hot.ee> wrote:
> It is not so bad as you put it with iterators (and pointers and
> references) to elements of 'map' (or to 'set' or to 'list') since the
> situations where those become invalidated are quite limited.
> The map to what the iterator is is also owned by object of 'DBE_Info'
> so I do not see harm of keeping some "bookmark" iterator to it.

There are some valid situations where you might want to keep an
iterator as a member variable (which will point to a member container),
but in that case you need to either disable the copy constructor and
assignment operator, or implement them properly (to, at the very
least, make the destination iterator point to the end of the destination
data container).

If the iterator is never used outside of a single member function call,
I don't really see a reason to keep it as a member variable. Keeping
it as variable only increases the likelihood of future mistakes, even
if the code is completely problem-free at this moment.