One event handler should be enough for anyone. (with detailed pants discussion)



  • From the source for GoodMerge, a small C# winforms app. This is from 2006, around the same time I started being a programmer for money, but even then this would have baffled me.

    private void InitializeComponent() 
    {
        ...
    
        EventListener = new EventHandler(eventHandler);
    
        ...
    
        Tabs.SelectedIndexChanged += EventListener;
        ButtonMerge.Click += EventListener;
        NumericMaximumRAM.TextChanged += EventListener;
        // snip a bunch more like this
    }
    
    private void eventHandler(object s, EventArgs e)
    {
        #region <Text Boxes>
        if (s == Textbox7ZipLocation)
        {
            options.SevenZip = Textbox7ZipLocation.Text;
            searchForProgs();
        }
        else if (s == TextboxAceLocation)
        {
            options.Ace = TextboxAceLocation.Text;
            searchForProgs();
        }
        else if (s == TextboxRarLocation)
        {
            options.Rar = TextboxRarLocation.Text;
            searchForProgs();
        }
        else if (s == TextboxWorkingFolder)
        {
            options.WorkingFolder = TextboxWorkingFolder.Text;
        }
        #endregion
        #region <List Boxes>
        else if (s == ListboxROMSet)
        {
        ...
        // snip a few hundred lines of varying complexity. the whole function is 350 lines of this.
    }
    

    There are around 40 controls on the main form. They all use the same event handler, which chooses an appropriate action based on the sender argument by way of the giant if-else.

    bonus wtf: I just learned you can stuff #region tags between if and else blocks. I had no reason to think that wouldn't be allowed, but I never thought to try it.



  • @fwd said:

    #region tags

    "You know what this preprocessor really needs? A tag that doesn't do anything. That's not a thing that anyone has been able to do before. Comments? What are those?"



  • Using #region to "comment block" sections isn't really that abnormal. Keep in mind the IDE is VS, so it will auto-magically collapse sections if you click the little - on the left hand side.

    Having said that, I think I saw delegate to a delegate which is TRWTF.



  • #regions are evil. They were invented to separate designer code from user code before partial classes were introduced, after that they had no purpose. The most common use of #region in real life is to attempt to hide really bad code.



  • I use #regions to hide my privates.


  • FoxDev

    Depends how you use them. I don't often use the region pragma, but when I do it is to group separate or very loosely coupled units of complexity in my code so you can easily collapse the parts of the code that you are not interested in at the moment without losing clarity.

    Of is a though line to draw, but I manage.... Sometimes. More often I just don't bother.





  • Yeah, it's all in how you use #regions. They can be helpful in grouping things that don't exactly fit into their own function. I use them in one client that fakes manual look up in a mainframe, so there are steps that should not be a function (to keep them from being called elsewhere when someone else is modifying the code). They are terrible for code smell but do have a use.



  • @accalia said:

    Depends how you use them

    Through VS2010, search didn't look in regions unless you changed the search options. That's been changed, but the ten years of people putting stuff in regions that weren't searched by default has jaded me severely. Stuff in regions still evades a quick eyeball scan, and that generally isn't a good thing.

    I'm nearly certain that if partial classes existed in .Net 1.0, regions would have never been invented. There's just isn't that much of a need for it. That's why no other major language has anything like it. I'm not talking about an IDE that does comment based folding, but an actual language pragma that does code folding.



  • Is the final else return DefWindowProc(hwnd, uMsg, wParam, lParam);?

    Because this looks suspiciously like it was written by someone used to writing handlers for the C Windows API.



  • Hah, I thought the same thing. Someone's still stuck in the I-don't-know-when-was-it-but-let's-not-go-back-there days.

    @locallunatic said:

    #regions

    They're not that bad, mostly in things that you can't separate (enums, or some large predefined dictionaries when you're not using a proper database).



  • I mostly use pants.


  • Discourse touched me in a no-no place

    @fwd said:

    There are around 40 controls on the main form. They all use the same event handler, which chooses an appropriate action based on the sender argument by way of the giant if-else.

    I've seen that sort of thing in Java programs too. Anti-pattern. Kill it with fire.



  • Java until recently (before version 8) didn't have a way for creating listeners from methods, so either you had to create a bunch of inner classes, use somewhat verbose anonymous classes or just make the object implement the relevant listener interfaces itself and have big if-chain by sender in the methods. Many programmers opted for the later. Many introductory texts did the same. It's a problem of introductory texts that the code they include is short, so the antipatterns don't stand out.


  • Discourse touched me in a no-no place

    @Bulb said:

    Many introductory texts did the same.

    And you'll find that those introductory texts were written back in the stone age pre-JDK 1.1 time when that was what you had to do. But it ceased to be good practice back in 1997, before C# was even generally available…



  • The trouble is that the bad texts stay around and the greenhorns can't tell which ones are good and which ones are not. Made worse by the fact that high school teachers of introductory courses often don't know either. Because who knows does it, who doesn't know teaches.



  • @Bulb said:

    who doesn't know teaches

    who doesn't know teaching teaches gym,

    and who doesn't know teaching gym teaches when the real teachers are off sick.



  • Don't say Java, because this event handling thing is only in AWT and Swing. JavaFX doesn't work like that for example:

    @FXML
    void quit(ActionEvent event) {}
    


  • JavaFX may have more reasonable method of binding handlers, but there are other libraries that use observer pattern and can provoke the antipattern then AWT and Swing.

    On Android it is often made worse by the fact that the sender is often separate process and only represented by identifier and there is no way to bind to specific sender, so you have to do the dispatch yourself. Of course this is problem of the API design more than language design.


  • I survived the hour long Uno hand

    @MathNerdCNU said:

    Keep in mind the IDE is VS, so it will auto-magically collapse sections if you click the little - on the left hand side

    Eclipse can do that on any block. I usually collapse my javadocs



  • @Yamikuronue said:

    Eclipse can do that on any block. I usually collapse my javadocs

    Most editors can do this. Regions provide collapsibility where there is no block.

    I'm OK with the collapsibility. I'm not OK with how it clutters up the code when it's expanded or how it collapses by default.



  • I use regions because my brain suffers trying to navigate a dynamically shaped concept procedurally.

    Ironic isn't it.

    All the OO code in the world can't help us avoid having to formulate the concepts in a procedurally generated fashion.

    Regions help me compose the navigational paths, so I can mentally move left and right, even though I'm just collapsing sections that are stacked vertically.



  • @Groaner said:

    Is the final else <code>return DefWindowProc(hwnd, uMsg, wParam, lParam);</code>?

    <small>Because this looks suspiciously like it was written by someone used to writing handlers for the C Windows API.</small>

    not in the event handler, but there is this:

    protected override void WndProc(ref Message m)
    {
        if (m.Msg != 0x0010) base.WndProc(ref m);
        else
        {
            // ...
            base.WndProc(ref m);
        }
    }
    

    apparently for catching the close button on the title bar.



  • Yay for magic numbers.



  • here's another interesting snippet. this is assembling a some how-to-use text that gets displayed in its own dialog. there are around 100 lines of this. This is apparently how the app does language localization.

        this.TextArea.SelectionFont=UnderlineFont;
        this.TextArea.AppendText("\n"+options.Strings[94]+":\n");
        this.TextArea.SelectionColor=OptionsColor;
        this.TextArea.AppendText("sf={"+options.Strings[161]+"}");
        this.TextArea.SelectionColor=NormalColor;
        this.TextArea.AppendText(": "+options.Strings[102]+" - "+options.Strings[105]+" "+options.Strings[160]+" \"<"+options.Strings[149]+">ren\".\n");
        this.TextArea.SelectionColor=OptionsColor;
        this.TextArea.AppendText("of={"+options.Strings[161]+"}");
        this.TextArea.SelectionColor=NormalColor;
        this.TextArea.AppendText(": "+options.Strings[106]+" - "+options.Strings[107]+" "+options.Strings[160]+" \"<"+options.Strings[149]+">Merge\". "+options.Strings[164]+"\n");


  • @fwd said:

    This is apparently how the app does language localization.

    Somehow numerically indexed (usually with symbolic constants, but still) still thrive all over the place, but I don't understand how anybody ever manages to maintain it. Because as the strings in developers' language evolve it provides absolutely no indication which translations need updating.



  • @another_sam said:

    I mostly use pants.

    I use your mom's mouth.



  • I wouldn't go so far as evil, but code-smell that is probably shit; that I can get behind.

    Reminds me, there's some #region-ed code I need to look at flushing. Sometimes I really hate "EVERYTHING is-an Object polymorphism"...



  • Dude, my mum's a grandma! Gross!



  • @fwd said:

    ```
    protected override void WndProc(ref Message m)
    {
    if (m.Msg != 0x0010) base.WndProc(ref m);
    else
    {
    // ...
    base.WndProc(ref m);
    }
    }

    
    And <code>0x10 == WM_CLOSE</code>. How cute.  I guess the author thought it best to catch all messages just in case Microsoft's *sloppy* <code>Form</code> implementation left out a few.


  • @another_sam said:

    Dude, my mum's a grandma!

    ....

    I know.

    😊



  • protected override void OnClosing(CancelEventArgs e) { /* ... */ }


    Filed under: There, was that so hard?, we need a new tag cloud to attack


  • Discourse touched me in a no-no place

    @fwd said:

    There are around 40 controls on the main form. They all use the same event handler

    I think I did that in Java once, for a form that only had about two controls. I think I even had a reason for it at the time, but that reason might have been "to see if I could". Either that or the controls had similar-enough functionality it sort of made sense to do that.


  • Discourse touched me in a no-no place

    @another_sam said:

    I mostly use pants.

    ...not trousers?

    ETA: Oh, wait, is that only a British thing?



  • @FrostCat said:

    Oh, wait, is that only a British thing?

    It's an English thing. You wouldn't understand.


  • Discourse touched me in a no-no place

    @another_sam said:

    It's an English thing. You wouldn't understand.

    Well, that's a dumb thing to say, given the context.



  • @FrostCat said:

    Well, that's a dumb thing to say, given the context.

    Let me add a word to make the joke more obvious: It's a [Queen's] English thing. The language, not the nationality. The language that you don't understand, because you speak American.

    Geez, it's not funny when I have to explain it.



  • Pants comes from pantaloons, which is a word that we American's didn't make up.


  • Discourse touched me in a no-no place

    @another_sam said:

    It's a [Queen's] English thing.

    Fine, that clarifies.

    @another_sam said:

    The language that you don't understand, because you speak American.

    I know what it means, you big dummy, or I wouldn't have said "not trousers" because I know how the English usage of those words differs from the American one. I didn't know that was Australian usage too.



  • @FrostCat said:

    I know what it means, you big dummy

    Yes, I know this, you big dummy. I was making a joke, which is now beyond dead. The old Australians are British poke → British vs English (nationality) → English (language) → American English vs. English English.

    Yes, to Australians pants usually means trousers, not underpants. In most differences between British English and American English we follow the British usage.


  • Discourse touched me in a no-no place

    @another_sam said:

    Yes, to Australians pants usually means trousers, not underpants.

    You could have avoided all this by just saying that in the first place, since you knew that I knew what you meant by my very action of asking.


  • :belt_onion:

    @Jaime said:

    American's

    !?
    twitch


  • Discourse touched me in a no-no place

    @another_sam said:

    In most differences between British English and American English we follow the British usage.

    The word 'pants' used the American way (for trousers) in some parts of Britain, and the 'British' English way (for underpants) in other parts.
    :wtf:



  • @FrostCat said:

    You could have avoided all this by just saying that in the first place, since you knew that I knew what you meant by my very action of asking.

    But then I couldn't have made my hilarious "Americans are dumb and don't understand English!" joke!



  • @another_sam said:

    Yes, to Australians pants usually means trousers, not underpants. In most differences between British English and American English we follow the British usage.

    But, it's the Brits that are wrong. As pointed out earlier, pants is a shortened form of pantaloons, which were generally down to the ankle. You guys decided to misapply the word. We American's's's's' are doing it right.

    The only way you could be right is if you unilaterally decide that all British usage is correct. In this case, you don't have an argument and we should ignore you.


  • Discourse touched me in a no-no place

    @another_sam said:

    But then I couldn't have made my dumb "Americans are dumb and don't understand English!" joke!

    FTFY.



  • @Jaime said:

    We American's's's's' are doing it right.

    Isn't pants underpants for Americans? That's not doing it right.

    For Australians pants means trousers, which are generally down to the ankle. Same as pantaloons, which is the funniest and stupidest word I've been reminded of in some time.


  • Discourse touched me in a no-no place

    @Jaime said:

    The only way you could be right is if you unilaterally decide that all British usage is correct.

    Hey, that's the usual argument used. By metric weenies, and xiB weenies, too!


  • Discourse touched me in a no-no place

    @another_sam said:

    Isn't pants underpants for Americans?

    Of course not. You put underpants under...wait for it...pants.

    Either you're trolling or you're asking for a whoosh flag on your original reply to me, because me using "pants" and "trousers" should have made it obvious I knew they weren't the same thing.


  • I survived the hour long Uno hand

    @another_sam said:

    For Australians pants means trousers, which are generally down to the ankle. Same as pantaloons, which is the funniest and stupidest word I've been reminded of in some time.

    This is the same as America


Log in to reply