LambdaButton

Here is a means of replacing the buttonClicked() Button::Listener api with std::function<>. Thanks to @basteln for the template concepts in his varx addition.

First, a means of making sure only Buttons are passed to the template

#include <type_traits>
#include <functional>
#include <utility>

namespace Is_A {
  template<typename T>
  using IsButton = typename std::enable_if<std::is_base_of<juce::Button, T>::value>::type;
}
//a Generic declaration we can specialize just for Buttons
template<typename T, typename Enable = void> class LambdaComponent;

now the template similar to basteln’s varx template to make juce components have React extensions

template<typename ButtonType>
class LambdaComponent<ButtonType, Is_A::IsButton<ButtonType>> : public ButtonType
{
public:
    std::function<void(Button* b)> buttonClickedHandler;        //this is the buttonClicked lambda
    std::function<void(Button* b)> buttonStateChangedHandler;   //this is the stateChanged lambda
    
    template<typename... Args>
    LambdaComponent(Args&&... args)
    :   ButtonType(std::forward<Args>(args)...),                           //this creates the button
    listener(this, buttonClickedHandler, buttonStateChangedHandler )    //this creates the listener
    {
        this->addListener(&listener);                                       //this links them
    }
private:
    class Listener : public ButtonType::Listener
    {
    public:
        Listener(Button* targ,
                 std::function<void(Button* b)>& clickHandler,
                 std::function<void(Button* b)>& stateHandler)
        : target(targ), buttonClickedHandler( clickHandler), buttonStateChangedHandler( stateHandler )
        {}
        ~Listener()
        {
            if( target != nullptr ) target->removeListener(this);           //this unlinks them
        }
        
        void buttonClicked(juce::Button *b) override
        { if( buttonClickedHandler ) { buttonClickedHandler(b); } }
        void buttonStateChanged(juce::Button *b) override
        { if( buttonStateChangedHandler ) { buttonStateChangedHandler(b); } }
    private:
        Button* target{nullptr};
        std::function<void(Button* b)>& buttonClickedHandler;
        std::function<void(Button* b)>& buttonStateChangedHandler;
    };
    Listener listener;
};

usage example:

LambdaComponent<TextButton> textButton;
textButton.buttonClickedHandler = [this](Button*) { DBG( "you clicked the button " + this->textButton.getName() ); };

much easier than the whole ; Button::Listener::buttonClicked(Button* b) derived class, button.addListener(this); if( b == &button )
 system currently in use!

2 Likes

Yes
 just be careful if *this ever goes out of scope, or you move/copy construct whatever *this was.

Nice. That’s a good “middle ground” if you just want the lambdas, and don’t want/need Rx.

Yes
 just be careful if *this ever goes out of scope, or you move/copy construct whatever *this was.

Could you explain which case you have in mind? Button is neither copy- nor moveable. And in the LambdaComponent class, the Listener has a shorter lifetime than the Button.

2 Likes

I think this class is great. I will definitely consider using it, but as someone who has only more recently got into complicated c++, I think newer C++ is tragic.
If this is only the beginning of the evolution of C++ then it may become an out of control monster. I can read the code, but if I was starting out again, I don’t know where I would start. The bar is way too high for a newcomer to ease his way into learning C++.
If C++ has to become like machine language to be fully usable, does that mean it started out with some wrong assumptions like linear compiling?
The frustration is that I have to continually change whole chunks of code just to keep up with the latest C++ suggestions.
Shouldn’t a clear and concise communicator be designing C++. Do the people who write that useless far too cleaver sample code in the text books, write the C++? Give me a small chat on stackoverflow over a text book anytime.
I feel better now I have had a rant.
Thanks for the sample code.

1 Like

Even if it’s off topic I agree with you on all counts, but one thing I didn’t get was



like what? From my own experience I was under the impression that for almost all of JUCE so long as you had a compiler which supported whatever C++ standard was required (for bits of JUCE which use new features under the hood) things can still be used in a very C++98 way. I can’t think of anything off the top of my head where an existing interface was code-breakingly changed to require one of the new C++11 features


Also, very cool class @matkatmusic. Really neat awesome use of templating to support all button class types!

Thanks! Check out my other thread, LambdaMouseListener, which was
originally written as another template specialization of this thread’s
template, but posted as a “class-ified” version. The format is pretty easy
to follow to create specialized templates for label, combo box, etc


I think this class is great. I will definitely consider using it, but as someone who has only more recently got into complicated c++, I think newer C++ is tragic.

I can relate to that feeling, though I like where C++ is going overall. Maybe I can share some ideas about the code above. My rationales for writing code like this are:

I compromise a bit of readability on my end (in the library), so that the library users can write more readable code. In moderation, of course. For example, instead of having to write LambdaButtonComponent<MyButton>, LambdaLabelComponent<MyLabel>, etc., the user just writes LambdaComponent<MyButton>, LambdaComponent<MyLabel>, etc. For that to work, we need this generic class declaration, and then the enable_if part, which can look a bit confusing.

Another part that may look strange at first:

template<typename... Args>
    LambdaComponent(Args&&... args)
    :   ButtonType(std::forward<Args>(args)...),

The goal here is: Inherit all of ButtonType’s constructors, but also do some extra work in the constructor. I would gladly write that in some other way, if possible. But in the meantime, I get used to this particular code pattern. When I see it, instead of reading word by word, I think “Okay, it passes all the arguments through, and then does something on top of that”.

Maybe this helps to understand why a lot of modern C++ looks like this. Now back to topic :slight_smile:

That was a bit hastily written. I will go back and edit the post. I should have said I keep discovering new and better ways to do things, such as the linking up buttons above and as I learn more I keep going back and changing on my code. I was left with a mess of code by a programmer and have started again and I keep starting again and again. This is not JUCE’s fault at all, just my journey into the complexities of C++. JUCE has only got better over time. Love it.
I was already working on a complex valuetree setup and I am so excited by the large projects and valuetree talk. I am going to go back and I will change my code further again, but I do love it. Meanwhile my old plugin users think I am dead.
C++ is easy with the benefit of hindsight, but some of the lamda linking ends up looking and reading like crazy. And all of this because a modern computer can’t put a link on hold. I have been implementing selecting and mouse events across multiple components where the info had to be sent up a number of parent classes and then back down to other components. Can’t compilers have macros where you say the function, tell it the the different scope and the class or implementation it is in, and it all links up. C++ is all based on the idea of inherentance and how human language evolves and gets more complicated in a forward time linear fashion. Let me tell thee that language doth evolve and the fundamental parent language inheriteth back from the changes unlike C++.
I also understand the need to make private and protect data from being accidentally accessed, but with C++ I feel like I have to break into my own house to use the toilet.
The templating is very handy, but it is very complicated looking considering it is really kinda a whole lot of #defines with no pragma onces in a big switch statement. Yet again once I got it, it was ok, but the first time you read it, it is code not language. An extension of MACROs may have been a better way to go.
With all the new C++ stds, castings, ways of holding data, reading data, etc the programmer should be telling the compiler what he wants to achieve, not how to achieve it. If telling the compiler what to achieve leaves the compiler with a choice then the compiler could help you provide extra information to fill out the code. AI programming is teaching us not to box anything into a corner as you do not know what that information may have to be used by or represented as. C++ is acknowledging this with auto and the like, but some of the twisting one has to do in C++ gets crazily hit or miss specific, which goes against the AI capabilities of the computer. I do not care so much how something is achieved, but I would rather achieve it.
I am thinking a good implementation of valuetree may be the solution to a lot of my problems. One container to rule them all. Love Juce

This is a really over-complicated approach, and inheritance is just the wrong tool for the job. TBH this is the kind of thing that makes a lot of people afraid of C++, and the reason people criticise OO programming in general.

Sure, sometimes horrible template nastiness is the only way to make something work. But in this case just a standalone version of that inner Listener class would be a better approach to the problem than trying to wrap it up with all that boilerplate. Leave the user’s base class alone, and let them use composition to attach things like this, not inheritance!

1 Like

thnks


actually, just thinking about it, an even more hands-off solution could be like this:

void attachCallback (Button& button, std::function<void()> callback)
{
    struct ButtonCallback  : public Button::Listener,
                             private ComponentListener
    {
        ButtonCallback (Button& b, std::function<void()> f) : target (b), fn (f)
        {
            target.addListener (this);
            target.addComponentListener (this);
        }

        ~ButtonCallback()
        {
            target.removeListener (this);
        }

        void componentBeingDeleted (Component&) override { delete this; }
        void buttonClicked (Button*) override  { fn(); }

        Button& target;
        std::function<void()> fn;

        JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (ButtonCallback)
    };

    new ButtonCallback (button, callback);
}
3 Likes

This is very similar to the idea I presented last year at ADC (Using Modern C++ to Improve Code Clarity (see slide 30).

The only real difference is that I sneakily hid it in the Component properties.
Jules’s method is probably a bit tidier though as if someone makes a copy of the Component properties with my method the callback would be kept alive (even though it wouldn’t be called due to the SafePointer usage).

But generally the idea of using composition here and attaching callbacks is much better than creating new component template types and is much easier to add to existing code.

1 Like

So, a global static method instead of adding the feature to the class that, imho, should’ve been there from the beginning? how many other GUI APIs come out of the box with allowing lambdas to be used as callbacks instead of listeners?

@dave96 Why aren’t ALL of those things presented in your talk part of JUCE by default? yeesh, that’s good stuff

like @basteln said, the goal is to make the USER experience with the API as easy as possible. It seems like there are a lot of times where the JUCE api was written expecting the user to come up with those fancy ‘structs-inside-global-functions’ themselves to get around the cumbersome approach the library defaults to. Just include it from the jump! My/basteln’s template solution is an answer to those of us who don’t want static global functions sitting around in our code, still want to be able to button.addListener(this); and also want to be able to button.buttonClickedHandler = []() { DBG("Awesome"); };
Sure, it might (for me at least) have been created out of a lack of experience programming, but the API design for responding to GUI events involves a whole lot of things to remember.
You gotta make a listener first ( : public <class>::Listener)
you gotta override pure virtual callbacks ( <class>::Listener::buttonClicked(Button*) = 0; )
then you gotta see which button was clicked ( if(b == &goButton ) { } else if( b == &pauseButton ) {} else if... )
That’s a lot to remember if you’re new! And it also forces you to jump around to implement the logic for an event, and remember where that implementation was when the project progresses, instead of just adding it right where it is being used. Some of those if() trees can get really long!

ScopedPointer<TextButton> myButton; 
...
myButton = new TextButton("hiiiidey hoo");
myButton->buttonClickedHandler = ...
or 
myButton->onClick = ...

that is sooo much easier to remember than the way juce has forced us for years. I don’t know, y’all are way more experienced than dudes like me, so solutions like what dave or jules shared might seem insignificant to come up with for you guys, but for the rest of us, they just seem like they should already exist in the library. Again, making the user experience with the API as easy as possible. Does Traction have huge if-trees for handling the transport bar buttons? Or did you say “F this listener nonsense” and replace it with that lambda approach as soon as C++11 came out?

Well I can’t speak on JUCE’s behalf but I would guess that adding such a pervasive set of C++11 features to the normal class APIs would be difficult. As far as I can tell JUCE still supports C++98 targets so I think the modern code is going to be phased in over the next few years as this legacy support is dropped. At the moment, newer APIs such as the DSP module or FlexBox/Grid are entirely wrapped in ifdefs, smaller sections of code such as initialiser list and move constructors are also ifdefed as they’re mostly just optimisations to be taken advantage of on newer platforms.

Changing the callback mechanism like this is a big paradigm shift and will probably need to be thoroughly thought out (as this thread proves there are at least several ways to accomplish these tasks). Using these modern techniques already requires some working knowledge of lambdas, capture lists, lifetime etc. so there is some benefit in making sure they are used safely and the implications properly understood.


What I have tried to show here (and indeed for the past couple of years in my talks) is that it’s often better to think outside the box for your solutions. Relying on JUCE for everything means you will always be stuck without something. I’m hoping that by using the techniques I’ve discussed, users will be able to write much better code themselves. I do agree though, having some “formal” method of using lambda callbacks would be nice. I just think it’s not exactly clear what that should look like yet. Once something is in the API, it is very hard to rectify or modify later on. User provided code doesn’t have this problem.

I’ve also discussed in several other posts on here why non-member functions are a very good idea. They much more clearly separate responsibility. There’s actually a lot of stuff in JUCE I would prefer to see as non-member functions. I think one problem is that users are afraid of them because they don’t fully understand them. There’s no real difference between a non-member non-friend function compared to a member function that doesn’t rely on having access to the internals of a class so it’s better to write them as such. It makes it a lot easier to start to think about extending classes with your own functionality rather than getting annoyed when something doesn’t appear in the class API.

I’m not sure where your fear of having global functions in your code is coming from, have you seen how the STL is implemented? If you include any of that you’ll have non-member non-friend functions lying around. If you put things in a namespace so they’re not fully global you won’t get collisions which is the usual problem with global functions.

More and mode these days I’m moving stuff out of classes and using the type system to determine what functions to call. It’s a bit long to explain here but I might write my next talk on the subject



In Tracktion we have a mixture to be honest. New stuff gets added using lambdas, old code gets updated when those areas get modified but we haven’t gone through the entire code base and replaced everything with lambdas. That’s just not really feasible with such a large code base.

4 Likes

Not sure where you got the idea we were trying to argue against a lambda-based callback approach. Not at all, I think it’d be great, and we’ll be adding more and more of that kind of thing soon.

But pay attention to Dave’s points here, they’re very good. I didn’t suggest the free function above because I’m trying to avoid adding anything to the library to do this, I just thought it might be useful for you in the meantime, as you seemed to be going off down crazy inheritance tangents. And when we do add this kind of thing, it’ll probably be almost exactly like that free function (+ versions for other listeners too), as it’s a really neat way to add that functionality.

1 Like

yeah, your example should probably become a static method in the Button class, tbh. I’m surprised it isn’t one already, considering how concise it is. Basically all classes that have a Listener class should have a static method like that


oh, to be able to think like you guys so that those types of code designs pop into my head when i’m tackling problems in my projects.

A suggestion:

  1. Move the addListener call to the Listener() constructor (i.e. targ->addListener(this);).
  2. Declare your listener member like this: Listener listener{ *this, buttonClickedHandler, buttonStateChangedHandler };
  3. Replace the entire LambdaComponent() constructor with: using ButtonType::ButtonType;

This way you can get rid of the Args&&-forwarding stuff.

Well, it’s not the component itself I’m worried about. Problem is std::functions from lambdas make it impossible to debug lifetimes captured in the lambdas, because the functions are type-erased, and seemingly legal and sensible modern C++ creates subtle bugs.

Given something like this:

template<typename Button>
struct MessageButton
{
	MessageButton(const std::string_view message)
		: msg(message), button(std::make_unique<LambdaComponent<Button>>())
	{
		button->onButtonClicked = [this] { MessageBox("You clicked the button!", msg.c_str()); };
	}

	std::string msg;
	std::unique_ptr<LambdaComponent<Button>> button;
};

And then this


std::vector<MessageButton> buttonList;

This would be a bug:

buttonList.emplace_back("hello");
buttonList.emplace_back("world"); // oops, iterators probably invalidated now and [this] of *begin() has changed, leading to memory corruption.

And even if you’re careful and use .reserve(), you could make the mistake of doing this:

buttonList.emplace_back(MessageButton("!")); // also a bug, r-value move constructor

Or this, if you wanted to create a function that constructs a list buttons and returns them:

return buttonList; // also a bug, implicit NVRO

Similarly


template<typename T>
MessageButton<T> MakeButton(const std::string_view message)
{
	return { message }; // also a bug, forced move-RVO
}

And something more obvious in this case (capturing a local by reference):

template<typename Button>
void addMessage(Button& b, std::string message)
{
	b.onButtonClicked = [&] { MessageBox("You clicked the button!", message.c_str()); }; // also a bug
}

etc. etc. The real issue is that none of these cases are detectable nor produce diagnostics (well maybe except the last one being really obvious), and only reproduce in certain circumstances non-local to the code that’s faulty, with no way of tracking/debugging it.

Thanks, I appreciate the explanation.

I don’t think the lambda capturing is to blame. Let’s say we didn’t use LambdaComponent<Button>, but a plain juce::Button. In the MessageButton constructor, you would have button->addListener(this) instead of the lambda. That creates the same problems with std::vector and moving, right? And I’m not sure how it would be easier to debug.

Well, I never said the alternative was better or worse. I just indicated a new solution could avoid/solve such problems.

With regards to debugging, the difference is you readily have the original [this] pointer available in the component’s listener list for tracing and comparison. No such thing exists for the std::function, because its contents are type erased.

If you used addListener, you would pair it up with a removeListener in the destructor (presumably). At that point, the unique_ptr would have been moved and be null, so you would get an instant crash at the problematic location (which is a good thing).

Yes, you could try to reset the std::function, but then the code needed is getting closer to the standard juce solution anyway, and since none of the presented example did this, I issued the “be careful” warning in the start of this topic (because it is not obvious you still have to do this, compared to an add/remove pair).

2 Likes