Returning a STL Map Object

Discussion in 'C++' started by anand, May 28, 2005.

  1. anand

    anand Guest

    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;
    }


    [ See http://www.gotw.ca/resources/clcm.htm for info about ]
    [ comp.lang.c++.moderated. First time posters: Do this! ]
     
    anand, May 28, 2005
    #1
    1. Advertising

  2. "anand" <> wrote in message
    news:...
    >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
    --
    http://ivan.vecerina.com/contact/?subject=NG_POST <- email contact form
    Brainbench MVP for C++ <> http://www.brainbench.com




    [ See http://www.gotw.ca/resources/clcm.htm for info about ]
    [ comp.lang.c++.moderated. First time posters: Do this! ]
     
    Ivan Vecerina, May 28, 2005
    #2
    1. Advertising

  3. anand

    meadori Guest

    > 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().


    [ See http://www.gotw.ca/resources/clcm.htm for info about ]
    [ comp.lang.c++.moderated. First time posters: Do this! ]
     
    meadori, May 29, 2005
    #3
  4. 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::pair<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;
    }


    [ See http://www.gotw.ca/resources/clcm.htm for info about ]
    [ comp.lang.c++.moderated. First time posters: Do this! ]
     
    James Daughtry, May 29, 2005
    #4
  5. anand

    Neil Pearce Guest

    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;
    }


    [ See http://www.gotw.ca/resources/clcm.htm for info about ]
    [ comp.lang.c++.moderated. First time posters: Do this! ]
     
    Neil Pearce, May 29, 2005
    #5
  6. anand

    Anand Guest

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

    -Anand


    [ See http://www.gotw.ca/resources/clcm.htm for info about ]
    [ comp.lang.c++.moderated. First time posters: Do this! ]
     
    Anand, May 29, 2005
    #6
  7. "Anand" <> wrote in message
    news:...
    > 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
    --
    http://ivan.vecerina.com/contact/?subject=NG_POST <- email contact form



    [ See http://www.gotw.ca/resources/clcm.htm for info about ]
    [ comp.lang.c++.moderated. First time posters: Do this! ]
     
    Ivan Vecerina, May 29, 2005
    #7
  8. anand

    BigBrian Guest

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

    > 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.


    [ See http://www.gotw.ca/resources/clcm.htm for info about ]
    [ comp.lang.c++.moderated. First time posters: Do this! ]
     
    BigBrian, May 29, 2005
    #8
  9. anand

    Guest

    BigBrian wrote:
    > > > public:
    > > > DUTBus() { }
    > > > Bus getBus();


    > > 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.


    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.)

    --
    James Kanze GABI Software
    Conseils en informatique orientée objet/
    Beratung in objektorientierter Datenverarbeitung
    9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34


    [ See http://www.gotw.ca/resources/clcm.htm for info about ]
    [ comp.lang.c++.moderated. First time posters: Do this! ]
     
    , May 30, 2005
    #9
  10. "BigBrian" <> wrote in message
    news:...
    >> > public:
    >> > DUTBus() { }
    >> > Bus getBus();

    >> 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.


    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::pair
    and std::string object stored will be copied to a new instance
    and address.

    Cheers,
    Ivan
    --
    http://ivan.vecerina.com/contact/?subject=NG_POST <- email contact form



    [ See http://www.gotw.ca/resources/clcm.htm for info about ]
    [ comp.lang.c++.moderated. First time posters: Do this! ]
     
    Ivan Vecerina, May 30, 2005
    #10
  11. anand

    Guest

    Anand wrote:
    > 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.

    --
    James Kanze GABI Software
    Conseils en informatique orientée objet/
    Beratung in objektorientierter Datenverarbeitung
    9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34


    [ See http://www.gotw.ca/resources/clcm.htm for info about ]
    [ comp.lang.c++.moderated. First time posters: Do this! ]
     
    , May 30, 2005
    #11
  12. BigBrian wrote:

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

    >>
    >>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.
    >



    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

    >
    > [ See http://www.gotw.ca/resources/clcm.htm for info about ]
    > [ comp.lang.c++.moderated. First time posters: Do this! ]
    >
     
    Kyle Kolander, May 31, 2005
    #12
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Marcus
    Replies:
    2
    Views:
    592
    Marcus
    Dec 9, 2005
  2. Replies:
    2
    Views:
    556
    klaus hoffmann
    Feb 22, 2006
  3. kl
    Replies:
    7
    Views:
    1,291
    James Kanze
    Jan 1, 2008
  4. ittium
    Replies:
    4
    Views:
    317
    ittium
    Dec 8, 2011
  5. Luca Risolia

    STL map to STL vector

    Luca Risolia, Jan 13, 2014, in forum: C++
    Replies:
    32
    Views:
    375
    Seungbeom Kim
    Jan 18, 2014
Loading...

Share This Page