0

I am creating a Signal and Slot system with a design like this:

There is the signal and slot classes

    struct timeDoubleEvent
    {
    public:
        timeDoubleEvent(const std::string& dataName)
            :
            dataName(dataName)
        {}
        const std::string& getDataName() const { return dataName; }
        Event<hydraPtr<DataHandler::timeDouble>> update;
        void fire(const hydraPtr<DataHandler::timeDouble>& timeDouble) const { update(timeDouble); }
    private:
        const std::string dataName;
    };

    class timeDoubleSlot
    {
    public:
        timeDoubleSlot(const std::string& dataName)
            :
            dataName(dataName)
        {}
        const std::string& getDataName() const { return dataName; }
        virtual void onEvent(const hydraPtr<DataHandler::timeDouble>& timeDouble) = 0;
    private:
        const std::string dataName;
    };

The slot would eventually differ for various cases, so I am creating derived class of it nested inside something:

    class BaseClass
    {
        // forward declaration
        class timePriceSlot;

    public:
        BaseClass(const std::string& name,
            const std::vector<hydraPtr<EventHandler::timeDoubleEvent>>& dataEventVector,
            const size_t& warmUpLength)
            :
            name(name),
            dataSlotVector(initializeDataSlotVector(dataEventVector)),
            warmUpLength(warmUpLength),
            dataStorage(initializeDataStorage(dataEventVector)),
            timeStorage(initializeTimeStorage(dataEventVector))
        {}

    private:
        const std::vector<hydraPtr<timePriceSlot>> dataSlotVector;

        const std::vector<hydraPtr<timePriceSlot>> initializeDataSlotVector(const std::vector<hydraPtr<EventHandler::timeDoubleEvent>>&);
        const bool& checkAllDataReceived(const std::string& dataName);

        class timePriceSlot : public EventHandler::timeDoubleSlot
        {
        public:
            timePriceSlot(const std::string& dataName,
                BaseClass& parent)
                :
                timeDoubleSlot(dataName),
                parent(parent)
            {}
            void onEvent(const hydraPtr<DataHandler::timeDouble>& timeDouble);
            BaseClass& getParent() const { return parent; }
        private:
            BaseClass& parent;
        };
    };

(only showing the relevant bits) In particular, the signal-slot connection is done like this:

    const std::vector<hydraPtr<BaseClass::timePriceSlot>> BaseClass::initializeDataSlotVector(
        const std::vector<hydraPtr<EventHandler::timeDoubleEvent>>& dataEventVector)
    {
        std::vector<hydraPtr<BaseClass::timePriceSlot>> output(dataEventVector.size());
        for (size_t i = 0; i < dataEventVector.size(); ++i)
        {
            EventHandler::timeDoubleEvent thisTimeDoubleEvent = (*dataEventVector[i]);
            std::shared_ptr<BaseClass::timePriceSlot> thisTimePriceSlot = std::make_shared<BaseClass::timePriceSlot>(dataEventVector[i]->getDataName(), *this);

            output[i] = thisTimePriceSlot;
            thisTimeDoubleEvent.update.connect([&](hydraPtr<DataHandler::timeDouble>& timeDouble) { (*thisTimePriceSlot).onEvent(timeDouble); });
        }

        return output;
    }

I am calling the function with a client program like this:

        const std::string stockName = "BBH";
        EventHandler::timeDoubleEvent event1(stockName);
        std::vector<hydraPtr<EventHandler::timeDoubleEvent>> eventVector(1);
        eventVector[0] = std::make_shared<EventHandler::timeDoubleEvent>(stockName);
        const hydraPtr<Signal::DerivedClass> myMA = std::make_shared<Signal::DerivedClass>(stockName + "_MA", eventVector, 10);

        const std::vector<hydraPtr<DataHandler::PriceTimeSeriesDataStruct>> myData = getAggregatedData();
        const std::vector<double> priceVectorCopy = myData[0]->getPriceVector();
        std::vector<double>::const_iterator it_d;
        const std::vector<std::string> timeVectorCopy = myData[0]->getDateTimeVector();
        std::vector<std::string>::const_iterator it_s;
        for (it_d = priceVectorCopy.begin(), it_s = timeVectorCopy.begin(); 
            it_d != priceVectorCopy.end(); it_d++, it_s++)
        {
            const hydraPtr<DataHandler::timeDouble> timeDoubleTemp = std::make_shared<DataHandler::timeDouble>(stockName, (*it_s), (*it_d));
            event1.fire(timeDoubleTemp);
        }

(DerivedClass is derived from BaseClass, with a different implementation of one function not relevant here) However, I found that even though there is no build or run time error, nothing happens upon the event fire. In fact, the onEvent function is never visited. Have I done something wrong? Thanks in advance.

PF Chang
  • 47
  • 6
  • You should [use pointer members instead of reference members](https://stackoverflow.com/questions/892133/should-i-prefer-pointers-or-references-in-member-data) and [avoid `const` data members](https://stackoverflow.com/questions/38155699/are-there-advantages-of-using-const-member-variable-in-c). Using these introduce constraints to your `class` that prevent very desirable properties like value semantics. – François Andrieux Feb 05 '20 at 15:02
  • `fire` calls the `Event`'s `operator()` but that isn't shown. There is already so much code shown here, adding more will just turn people away from your question. Try to come up with a [MCVE] to illustrate the problem. – François Andrieux Feb 05 '20 at 15:06
  • 1
    One thing that jumps out at me is that `thisTimePriceSlot` is captured by reference (due to `[&]`). If `connect` stores the lambda for later use, what it's referring to won't exist anymore. – François Andrieux Feb 05 '20 at 15:08

1 Answers1

0

The problem was in this line

thisTimeDoubleEvent.update.connect([&](hydraPtr<DataHandler::timeDouble>& timeDouble) { (*thisTimePriceSlot).onEvent(timeDouble); });

Both the event and the slot should be pointers, not dereferenced values.

PF Chang
  • 47
  • 6