Returning a STL Map Object

[anand]

I am an STL newbie trying to build a class DUTBus that has a map
object. In my member function, I try to return a Map Object, however,
it does not seem to work. I am referring to "Bus getBus()"
function.

I need to do this because in order to "add" device pins (via
registerPin() function. In other words, I need to modify the map by
inserting new (key,value) pairs into the object. However, when I try to
access the bus object by returning it, it doesnt work!

Please help!

Thanks
Anand

#include <string>
typedef map<int, string> Bus;
typedef Bus::iterator BusIterator;

class DUTBus
{
private:
Bus _bus;
public:
DUTBus() { }
Bus getBus();
inline int getBusSize() { return (_bus.size()); }
void registerPin(int, string);

};


Bus DUTBus::getBus()
{
return(_bus);
}

void DUTBus::registerPin(int _idx, string _pin)
{
Bus myBus = this->getBus();
BusIterator iter;
iter = myBus.begin();
myBus.insert(pair<int, string>(_idx,_pin));
cout << "Inserted Bus[" << _idx << "]\t=\t" << _pin;
}




main(void)
{
Bus myBus;
DUTBus* DataBus = new DUTBus();
DataBus->registerPin(1,"T_x_p_ad1");
DataBus->registerPin(2,"T_x_p_ad2");
DataBus->registerPin(3,"T_x_p_ad3");

myBus = DataBus->getBus();
cout << "This bus has a size of " << myBus.getBusSize() << endl;
}

 

[Ivan Vecerina]

I am an STL newbie trying to build a class DUTBus that has a map
object. In my member function, I try to return a Map Object, however,
it does not seem to work. I am referring to "Bus getBus()"
function.

I need to do this because in order to "add" device pins (via
registerPin() function. In other words, I need to modify the map by
inserting new (key,value) pairs into the object. However, when I try to
access the bus object by returning it, it doesnt work!

Please help!

Thanks
Anand

#include <string>
typedef map<int, string> Bus;
typedef Bus::iterator BusIterator;

class DUTBus
{
private:
Bus _bus;

NB: Leading underscores are best avoided in identifier names
(they are reserved for the implementation in some contexts).
Trailing underscores (bus_) are therefore usually preferred.

public:
DUTBus() { }
Bus getBus();

Problem: In C++, this returns a (deep) copy of the stored _bus
object. If you want to return an object reference, you need to
do so explicitly:
Bus& getBus() { return _bus; }
However, the latter defeats the purpose of having a private
data member, and is therefore best avoided.
A sometimes useful alternative, which still allows the class to
control its invariants, is:
Bus const& getBus() const { return _bus; }

inline int getBusSize() { return (_bus.size()); }

NB: inline is redundant here, and a 'const' would be better:
int getBusSize() const { return (_bus.size()); }

void registerPin(int, string);

};


Bus DUTBus::getBus()
{
return(_bus);
}

void DUTBus::registerPin(int _idx, string _pin)\

Again, leading underscores are best avoided. And it is
a quite uncommon C++ style to use them in this context.

{
Bus myBus = this->getBus();

This creates a local copy of the _bus object. That local
copy is then modified, and destroyed, while the _bus data
member remains unchanged. ***this is what causes your 'bug'***
Use the _bus data member direcly. Or if you want a local
reference to the data member, use:
Bus& myBus = _bus;

BusIterator iter;
iter = myBus.begin();

Prefer combining declaration and instanciation:
BusIterator const iter = myBus.begin();
But anyway, 'iter' is not needed in this function.

myBus.insert(pair<int, string>(_idx,_pin));
cout << "Inserted Bus[" << _idx << "]\t=\t" << _pin;
}

The function will work correctly if you replace its body with:
_bus.insert( pair<int, string>(_idx,_pin) );
Or even easier, since you don't seem to care about wheter a pin
with the same number already existed:
_bus[_idx] = _pin;

main(void)
{
Bus myBus;

It is better to declare local variables on first use...

DUTBus* DataBus = new DUTBus();

Don't allocate objects on the heap unless it is necessary. Just use:
DUTBus DataBus;

DataBus->registerPin(1,"T_x_p_ad1");
DataBus->registerPin(2,"T_x_p_ad2");
DataBus->registerPin(3,"T_x_p_ad3");

myBus = DataBus->getBus();

This again creates a local *copy* of the DataBus->_bus , which
may not be what you want (although it will work ok in this context).

cout << "This bus has a size of " << myBus.getBusSize() << endl;
}

I hope this helps. Note that it might be a good idea to spend more time
studying C++ before trying to write more complex programs...


Regards, Ivan

 

[meadori]

I try to return a Map Object, however, it does not seem to work.
Could you please be a bit more specific about what doesn't work?
Compile time error? Unexpected runtime behavior?

As far as I can tell the only thing wrong with the above code is the
line:
cout << "This bus has a size of " << myBus.getBusSize() << endl;
.. You are calling getBusSize() on an object of type Bus. Type Bus is
really a map<int, string> and, of course, does not have a member
function getBusSize().

 

[James Daughtry]

You're confusing a Bus with a DUTBus, and the insertions don't work
because you're inserting into a local copy of _bus, not _bus itself.
Also, don't forget to include the requisite headers and qualify for the
std namespace:

#include <iostream>
#include <map>
#include <utility>
#include <string>

typedef std::map<int, std::string> Bus;
typedef Bus::iterator BusIterator;

class DUTBus
{
private:
Bus _bus;
public:
Bus getBus();
inline int getBusSize() { return (_bus.size()); }
void registerPin(int, std::string);
};

Bus DUTBus::getBus()
{
return(_bus);
}

void DUTBus::registerPin(int _idx, std::string _pin)
{
_bus.insert(std:air<int, std::string>(_idx,_pin));
std::cout << "Inserted Bus[" << _idx << "]\t=\t" << _pin;
}

int main(void)
{
Bus myBus;
DUTBus* DataBus = new DUTBus();

DataBus->registerPin(1,"T_x_p_ad1");
DataBus->registerPin(2,"T_x_p_ad2");
DataBus->registerPin(3,"T_x_p_ad3");

myBus = DataBus->getBus();
std::cout << "This bus has a size of " << myBus.size() << std::endl;
}

 

[Neil Pearce]

The problem is that during every iteration of registerPin(),
you are calling your getBus() function, which returns a copy
of your Bus map object.
Your insert operation acts on this copy, which is then lost
when the registerPin() function finishes.

So, to fix - change your registerPin() function so that it
directly inserts on the contained Bus object instead.

#include <string>
#include <iostream>
#include <map>

class DUTBus {
public:
typedef std::map<int, std::string> Bus;

const Bus& getBus() const { return bus; }

Bus::size_type getBusSize() const { return bus.size(); }

void registerPin(int index, const std::string& pin) {
bus.insert(std::make_pair(index, pin));
}

private:
Bus bus;
};

int main() {
DUTBus dataBus;
dataBus.registerPin(1, "T_x_p_ad1");
dataBus.registerPin(2, "T_x_p_ad2");
dataBus.registerPin(3, "T_x_p_ad3");

const DUTBus::Bus& myBus = dataBus.getBus();

std::cout << "This bus has a size of " << myBus.size() << std::endl;
}

 

[Anand]

You are basically saying that dont declare a local DataBus since it is
redundant and wastes memory...Hope I got it right!

-Anand

 

[Ivan Vecerina]

You are basically saying that dont declare a local DataBus since it is
redundant and wastes memory...Hope I got it right!

Nearly yes.
The key thing is that you need to understand better the C++ object
model: read about object constructors, copy-constructors, and
assignment operators; understand what references and pointers
mean in C++.
I would recommend "Accelerated C++" as a book [by Austern & Moo],
of if you are looking for something that is also available online
for free, Thinking in C++, by Bruce Eckel.

{Moderator's note: "Accelerated C++" is by Andrew Koenig and Barbara
Moo, not by Matt Austern; Matt's book is "Generic Programming and the STL".
-dk/mod}

Cheers,
Ivan

 

[BigBrian]

public:

Problem: In C++, this returns a (deep) copy of the stored _bus
object.

I don't think this is true. It returns a copy by calling the copy
constructor. If one is not implemented, it calls the compiler
generated default copy constructor which is *NOT* a deep copy.

 

[kanze]

I don't think this is true. It returns a copy by calling the
copy constructor. If one is not implemented, it calls the
compiler generated default copy constructor which is *NOT* a
deep copy.

But in this case, one is implemented, and it has deep copy
semantics. (Bus is a typedef to an instantiation of std::map.)

In general, deep copy is idiomatic C++, and except in special
cases, you should either ensure deep copy, or ban copy, in your
own classes. And the compiler generated copy constructor is
member-wise copy -- if all of the members implement deep copy,
it is effectively deep copy. (Of course, C++ raw pointers do
NOT have deep copy semantics. And the presence of a raw pointer
in a class is frequently a sort of a danger flag that says you
need a user defined copy contructor.)

 

[Ivan Vecerina]

I don't think this is true. It returns a copy by calling the copy
constructor. If one is not implemented, it calls the compiler
generated default copy constructor which is *NOT* a deep copy.

I added this '(deep)' qualification only to clearly oppose
this case to what happens in Java and other languages, where an
object is "copied" by just creating a new reference/pointer to it.

Furthermore, given that 'Bus' was defined as:
typedef map<int, string> Bus;
the copy of the object is as deep as it can be: each std:air
and std::string object stored will be copied to a new instance
and address.

Cheers,
Ivan

 

[kanze]

You are basically saying that dont declare a local DataBus
since it is redundant and wastes memory...Hope I got it right!

No. He's saying not to declare a local DataBus because it is a
separate object. Modifying it does not change anything in the
DataBus object it was created from.

Redundancy and wasting memory are optimization issues. In the
case of containers, you can't necessarily ignore them, but the
issue here is a more serious one of correctness: using deep copy
semantics when you need reference semantics.

 

[Kyle Kolander]

I don't think this is true. It returns a copy by calling the copy
constructor. If one is not implemented, it calls the compiler
generated default copy constructor which is *NOT* a deep copy.

You are correct, this is not a "deep" copy. Try storing pointers
to objects in your map and see how well it does a deep copy. All it
will copy is the address of each element in the map... the result is
two maps that both point to the same elements on the heap. A
modification in one map will affect the other. This is not deep copy
semantics.

Kyle