Thread: Constructor problem in list of classes

  1. #1
    Registered User
    Join Date
    Mar 2008
    Posts
    71

    Constructor problem in list of classes

    Hey all,

    I have a list of classes enclosed in a class:


    Code:
    /* The controling class of the whole program. Calls the functions responsible for displacement calculation and object rendering,
     * as well as containing the list of ClStar objects
     */
    
    #ifndef CLASS_UNIVERSE_H
        #define CLASS_UNIVERSE_H
    
    #include <list>
    #include <iostream>
    #include "ClassStar.h"
    #include "ClassValues.h"
    #include "ClassMath.h"
    #include "ClassGravCalculator.h"
    #include "ClassRender.h"
    #include "ClassPosRegulator.h"
    
    using namespace std;
    
    
    class ClUniverse {
        private:
    
        list<ClStar> m_Stars;
    
        ClPosRegulator m_ClPosReg;
        ClGravCalculator m_ClGravCalc;
    
        public:
    
        ClUniverse();
    
        void calculateDisplacement();
        void drawObjects();
    };
    
    #endif
    Code:
    #include "ClassUniverse.h"
    
    
    ClUniverse::ClUniverse() : m_Stars(ClValues::nNumObjects)
    {}
    
    void ClUniverse::calculateDisplacement()
    {
        list<ClStar>::iterator it_NxtStar;
        for(list<ClStar>::iterator it_CurStar = m_Stars.begin(); it_CurStar != m_Stars.end(); it_CurStar++) {
            it_NxtStar = it_CurStar;
            it_NxtStar++;
            while(it_NxtStar != m_Stars.end()) {
                m_ClGravCalc(*it_CurStar, *it_NxtStar);
                cout << it_CurStar->getPosition().getX() << endl;
                cout << it_CurStar->getPosition().getY() << endl;
                cout << it_NxtStar->getPosition().getX() << endl;
                cout << it_NxtStar->getPosition().getY() << endl;
                cout << endl;
                it_NxtStar++;
            }
            m_ClPosReg(*it_CurStar);
        }
    }
    
    void ClUniverse::drawObjects()
    {
        clear(ClValues::dblbuffer);
    
        drawing_mode(DRAW_MODE_TRANS, 0, 0, 0);
    
        for(list<ClStar>::iterator it_CurStar = m_Stars.begin(); it_CurStar != m_Stars.end(); it_CurStar++)
            ClRender(ClValues::dblbuffer, it_CurStar->getPosition(), it_CurStar->getRadius());
    
        vsync();
        blit(ClValues::dblbuffer, screen, 0, 0, 0, 0, ClValues::nScreenHight, ClValues::nScreenWidth);
    }
    Here are the ClStar files:

    Code:
    /* This file contains the declarations for ClStar. It contains variables to hold all the necessary data to simulate gravitation displacement
     * between stars using classical mechanics
     */
    
    #ifndef CLASS_STARS_H
        #define CLASS_STARS_H
    
    #include <cmath>
    #include "ClassMath.h"
    #include "ClassVector.h"
    #include "ClassValues.h"
    #include "ClassRange.h"
    
    class ClStar {
        private:
    
        //Mass
        long double m_dMass;
    
        //Density
        long double m_dDensity;
    
        //Radius
        long double m_dRadius;
    
        //Position vector
        ClVector m_vecPosition;
    
        //Velocity vector
        ClVector m_vecVelocity;
    
        //The number of stars
        static int s_nNumStars;
    
        //---------------------------------------------
    
        /*//Next node in linked list
        ClStar* m_nextnode;*/
    
        //---------------------------------------------
    
        public:
    
        //Constructor, will use vales from the class ClValues
        //ClStar(int nNumNodes);
        ClStar();
    
        //Access functions
        long double getMass();
        long double getDensity();
        long double getRadius();
        ClVector getPosition();
        ClVector getVelocity();
    
        void setPosition(ClVector vecPosition);
        void setPosition(long double dX, long double dY);
        void addPosition(ClVector vecPosition);
        void addPosition(long double dX, long double dY);
    
        void setVelocity(ClVector vecVelocity);
        void setVelocity(long double dX, long double dY);
        void addVelocity(ClVector vecVelocity);
        void addVelocity(long double dX, long double dY);
    
        //ClStar* getNextNode();
    
    };
    
    #endif
    Code:
    #include "ClassStar.h"
    
    int ClStar::s_nNumStars = 0;
    
    /*ClStar::ClStar(int nNumNodes) : m_nextnode(0)
    {
        //Generate random values for mass and density, using ranges found in ClValues,
        //and compute the radius. This program treats the objects as spheres.
        m_dMass = ClMath::randNum(ClValues::rnMass);
        m_dDensity = ClMath::randNum(ClValues::rnDensity);
        m_dRadius = std::pow((m_dMass/m_dDensity) / (4 * ClMath::dPI / 3), static_cast<long double>(1.0/3.0));
    
        //Get the sequence number of current node. Starts at 1
        int nCurNode = ClValues::nNumObjects - nNumNodes + 1;
    
        //The screen will be divided into "quadrants" to lower the chances of two stars sharing coordinates. One quadrant will be as
        //high as the screen, and as wide as ScrWidth/NumObjects
        int nQuadrantWidth = static_cast<int>(floor(ClValues::nScreenWidth / ClValues::nNumObjects));
        ClRange<long int> rngWidth(nQuadrantWidth * (nCurNode - 1), nQuadrantWidth * nCurNode - 1);
        ClRange<long int> rngHight(0, ClValues::nScreenHight);
    
        //Generate coordinates
        m_vecPosition.setVector(static_cast<long double> (ClMath::randNum(rngHight)), static_cast<long double> (ClMath::randNum(rngWidth)));
    
        //Create new node if needed ammount has not yet been reached
        if(nNumNodes - 1)
            m_nextnode = new ClStar(nNumNodes - 1);
    
    }*/
    
    ClStar::ClStar()
    {
        //Generate random values for mass and density, using ranges found in ClValues,
        //and compute the radius. This program treats the objects as spheres.
        m_dMass = ClMath::randNum(ClValues::rnMass);
        m_dDensity = ClMath::randNum(ClValues::rnDensity);
        m_dRadius = std::pow((m_dMass/m_dDensity) / (4 * ClMath::dPI / 3), static_cast<long double>(1.0/3.0));
    
        //Number of the star
        int nCurNode = ++s_nNumStars;
    
        //The screen will be divided into "quadrants" to lower the chances of two stars sharing coordinates. One quadrant will be as
        //high as the screen, and as wide as ScrWidth/NumObjects
        long int nQuadrantWidth = static_cast<long int>(floor(ClValues::nScreenWidth / ClValues::nNumObjects));
        ClRange<long int> rngWidth(nQuadrantWidth * (nCurNode - 1), nQuadrantWidth * nCurNode - 1);
        ClRange<long int> rngHight(0, ClValues::nScreenHight);
    
        //Generate coordinates
        m_vecPosition.setVector(static_cast<long double> (ClMath::randNum(rngHight)), static_cast<long double> (ClMath::randNum(rngWidth)));
    }
    
    long double ClStar::getMass()
    {
        return m_dMass;
    }
    
    long double ClStar::getDensity()
    {
        return m_dDensity;
    }
    
    long double ClStar::getRadius()
    {
        return m_dRadius;
    }
    
    ClVector ClStar::getPosition()
    {
        return m_vecPosition;
    }
    
    ClVector ClStar::getVelocity()
    {
        return m_vecVelocity;
    }
    
    void ClStar::setPosition(ClVector vecPosition)
    {
        m_vecPosition = vecPosition;
    }
    
    void ClStar::setPosition(long double dX, long double dY)
    {
        ClVector vec(dX, dY);
        m_vecPosition = vec;
    }
    
    void ClStar::addPosition(ClVector vecPosition)
    {
        m_vecPosition += vecPosition;
    }
    
    void ClStar::addPosition(long double dX, long double dY)
    {
        ClVector vec(dX, dY);
        m_vecPosition += vec;
    }
    
    void ClStar::setVelocity(ClVector vecVelocity)
    {
        m_vecVelocity = vecVelocity;
    }
    
    void ClStar::setVelocity(long double dX, long double dY)
    {
        ClVector vec(dX, dY);
        m_vecVelocity = vec;
    }
    
    void ClStar::addVelocity(ClVector vecVelocity)
    {
        m_vecVelocity += vecVelocity;
    }
    
    void ClStar::addVelocity(long double dX, long double dY)
    {
        ClVector vec(dX, dY);
        m_vecVelocity += vec;
    }
    Problem is, all the "stars" get generated with exactly the same coordinates. Before, when I made my own linked list, the code worked fine. But when I rewrote the code to use lists instead, it doesn't work anymore. Any tips as to why this is happening?

    Cheers,

    Gabe

    PS.: Ignore anything that's commented out, it's just the old code using linked lists. And sorry for the debug stuff that's strewn all over the place.
    Last edited by G4B3; 05-29-2009 at 09:31 AM.

  2. #2
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Where do you call srand()?

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  3. #3
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    I also don't see where you add anything to the list?

  4. #4
    Registered User
    Join Date
    Mar 2008
    Posts
    71
    I call srand() in an initiation class that I haven't posted, but it get's called. It did before, and I haven't touched that code since I wrote it. As for adding anything to the list, shouldn't that happen here:

    Code:
    ClUniverse::ClUniverse() : m_Stars(ClValues::nNumObjects)
    {}
    ClValues::nNumObjects is an integer specifying, surprisingly, the number of objects. When I was trying it out it was 20.
    Last edited by G4B3; 05-29-2009 at 10:10 AM.

  5. #5
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    So, put a breakpoint (or some tracing code) in the star constructor.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  6. #6
    Registered User
    Join Date
    Mar 2008
    Posts
    71
    *blush* that is an embarrassingly obvious solution isn't it...

    Anyway, did that, and it did help a little bit. Turns out the constructor only gets called once, or so it seems. s_nNumStars is always == 1, and if I put a breakpoint in the constructor it only halts once...so I guess one object is initialized, and the rest are initialized by...I don't know, a hideous parody of a copy constructor I guess...but as for why it's doing that and how to solve it, I have no idea.
    Last edited by G4B3; 05-29-2009 at 10:21 AM.

  7. #7
    The larch
    Join Date
    May 2006
    Posts
    3,573
    Yes, in such cases the container is filled with copies of the same object.

    To solve it, you could push_back instances into the list in a loop.
    I might be wrong.

    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
    Quoted more than 1000 times (I hope).

  8. #8
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Quote Originally Posted by G4B3 View Post
    I call srand() in an initiation class that I haven't posted, but it get's called. It did before, and I haven't touched that code since I wrote it. As for adding anything to the list, shouldn't that happen here:

    Code:
    ClUniverse::ClUniverse() : m_Stars(ClValues::nNumObjects)
    {}
    ClValues::nNumObjects is an integer specifying, surprisingly, the number of objects. When I was trying it out it was 20.
    No? To add things to a list, you use push_back.

  9. #9
    Registered User
    Join Date
    Mar 2008
    Posts
    71
    Oh...I though the constructor was called for every object. So something like this?

    Code:
    ClUniverse::ClUniverse()
    {
         for(int x = 0; x < ClValues::nNumObjects; x++) {
              ClStar TheStar;
              m_Stars.push_back(TheStar);
        }
    }

  10. #10
    Registered User
    Join Date
    Apr 2008
    Posts
    890
    Any reason you're storing objects in your list instead of smart pointers to objects?

  11. #11
    The larch
    Join Date
    May 2006
    Posts
    3,573
    Any reason why you ask? The ClStar class is pure data, std::list only requires one copy made?
    I might be wrong.

    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
    Quoted more than 1000 times (I hope).

  12. #12
    Registered User
    Join Date
    Apr 2008
    Posts
    890
    Quote Originally Posted by anon View Post
    Any reason why you ask? The ClStar class is pure data, std::list only requires one copy made?
    But there's more copying going on if we're passing objects, and not references, in the bolded lines below, and since all other functions in the code snippet accept objects, I assumed.

    On a more theoretical note, by default I disallow copying on all classes, since semantically it usually makes no sense. In the real world we can't copy stars (or cars or trees or cats) but we seem to think it's a good idea in software for some reason.

    Code:
    void ClUniverse::calculateDisplacement()
    {
        list<ClStar>::iterator it_NxtStar;
        for(list<ClStar>::iterator it_CurStar = m_Stars.begin(); it_CurStar != m_Stars.end(); it_CurStar++) {
            it_NxtStar = it_CurStar;
            it_NxtStar++;
            while(it_NxtStar != m_Stars.end()) {
                m_ClGravCalc(*it_CurStar, *it_NxtStar);
                cout << it_CurStar->getPosition().getX() << endl;
                cout << it_CurStar->getPosition().getY() << endl;
                cout << it_NxtStar->getPosition().getX() << endl;
                cout << it_NxtStar->getPosition().getY() << endl;
                cout << endl;
                it_NxtStar++;
            }
            m_ClPosReg(*it_CurStar);
        }
    }
    Last edited by medievalelks; 05-29-2009 at 09:24 PM.

  13. #13
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Quote Originally Posted by medievalelks
    But there's more copying going on if we're passing objects, and not references, in the bolded lines below, and since all other functions in the code snippet accept objects, I assumed.
    However, just storing smart pointers in the container will not stop the unnecessary copying since the objects will still be passed by value after the smart pointers are dereferenced. An appropriate way to stop the unnecessary copying is to change the parameters in question to be reference parameters.

    Quote Originally Posted by medievalelks
    On a more theoretical note, by default I disallow copying on all classes, since semantically it usually makes no sense. In the real world we can't copy stars (or cars or trees or cats) but we seem to think it's a good idea in software for some reason.
    We are modeling the real world (if we are not modeling something more imaginary/abstract) rather than actually recreating the real world. As such, we can define what semantics are useful to us.
    Last edited by laserlight; 05-30-2009 at 04:18 AM.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  14. #14
    Registered User
    Join Date
    Mar 2008
    Posts
    71
    I was passing references in the original code, but I just haven't gotten around to rewriting it to use lists/iterators. The prototypes for the two highlighted functions are:

    Code:
    void ClGravCalculator::operator()(ClStar& curStar, ClStar& nxtStar)
    void ClPosRegulator::operator()(ClStar& Star)
    Although, now that I think of it, shouldn't the current code also work? Or is there some passing-by-value involved that I'm not seeing...?

  15. #15
    Registered User
    Join Date
    Apr 2008
    Posts
    890
    It will work, there's just a chance it will be a performance hit, especially if your universe is to scale :-). Pass by const &, too, unless of course you're modifying the ClStar in those calls.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Duplicating value of pointer to linked list
    By zephyrcat in forum C Programming
    Replies: 14
    Last Post: 01-22-2008, 03:19 PM
  2. Please Help - Problem with Compilers
    By toonlover in forum C++ Programming
    Replies: 5
    Last Post: 07-23-2005, 10:03 AM
  3. Replies: 3
    Last Post: 03-04-2005, 02:46 PM
  4. problem with structures and linked list
    By Gkitty in forum C Programming
    Replies: 6
    Last Post: 12-12-2002, 06:40 PM
  5. Contest Results - May 27, 2002
    By ygfperson in forum A Brief History of Cprogramming.com
    Replies: 18
    Last Post: 06-18-2002, 01:27 PM