Is this an anti-pattern/code-smell?



  • Today I got into a discussion about this pattern in the code I'm currently working on:

    function x(params) {
        doX(params.a, params.b, param.c);
    }
    x({a:1, b:2, c:3});
    

    Versus this:

    function x(a, b, c) {
        doX(a, b, c);
    }
    x(1,2,3);
    

    Why I find this annoying:

    1. Harder to write (what does exactly this function wants?)
    2. Harder to write (you have to check if all the function needs is in the params)
    3. Harder to read
    4. Harder to debug (check everything is in params)

    Am I TRWTF?



  • While I see some potential benefits to the first form (optional arguments?) I do agree the second one should be the default choice. No clue why you'd go for the first by default.


  • Discourse touched me in a no-no place

    Is

    parms = {a:1, b:2, c:3}
    
    x(params);
    y(params);
    x(params);
    

    a thing in that language (or perhaps more pertinently) your application?


  • sockdevs

    rule of thumb i work with:

    if two or more of the following apply use a strongly typed (if available in the language) object to pass the parameters.

    • There are more than 5 parameters (if more than 10 this one alone is sufficient)
    • some of the parameters are optional
    • the parameters are "grouped" into logical entities
    • the parameters need to be passed in bulk to a chain of methods


  • Yes, but not necessarily all x,y,z use the same variables in params. At the end we have like 50 different functions receiving (and modifying) the params object and using different values. For example:

    function y(params) {
        doY(params.a params.k)
       // where did k came from?
    }
    

    I also see lots of this:

    function z(params) {
        params.u = doSomething()
        doZ(params)
    }
    
    function doZ(params) {
        // use params.u and params.a
    }
    


  • I agree, this is the sane way of doing this, but it's important that the object you're referring to would probably be a model defined elsewhere. Here it's basically a clusterfuck of associative arrays defined on run time.

    And most of the times, this functions don't reach the 5 params mark (I also feel this as a reasonable limit to think: maybe I should split this function or write an object)



  • Sounds like someone just loves them some json. I believe some people think they are more advanced or something when they do this. It's like abusing closures or any other feature of the moment. You latch onto something that is interesting and new and overuse it.

    Then I come back to it a few months later and am embarrassed at my old infatuations.


  • Discourse touched me in a no-no place

    @Eldelshell said:

    Yes, but not necessarily all x,y,z use the same variables in params. At the end we have like 50 different functions receiving (and modifying) the params object and using different values.

    Kill it with :fire: then.

    If it was something akin to C's structs then I could maybe understand it, but otherwise it seems to be doing boilerplate for the sake of doing boilerplate.



  • Sad thing, I spent an hour last night fixing a small portion of this only to be slapped in the face by the hand of git merge from a coworker who uses this pattern, so I'm basically giving up and venting my frustration here.

    The only thing I can think of why this was done this way is some limitation of the PL we're using:

    //This works
    doX({
      a: 1,
      b: 2,
      c: 3
    })
    
    // Syntax error
    doX(
       1,
       2,
       3
    )
    


  • @Eldelshell said:

    The only thing I can think of why this was done this way is some limitation of the PL we're using:

    ZOMG. What language is this?



  • It looks like javascript, you'd expect it to be javascript. If it's some custom language there's no telling what's going on...

    I did realize there's an additional reason to do something like this: If you're supplying a callback function and are only allowed 1 passthrough argument.


  • sockdevs

    @accalia said:

    * some of the parameters are optional

    Depends on the language; C# has had optional method parameters from version 4, and they come in handy occasionally.

    And no, it doesn't auto-create overloads; it inlines the default parameter values at the call-site, so your API remains the same :wink:



  • @PleegWat said:

    It looks like javascript, you'd expect it to be javascript.

    // Syntax error
    doX(
       1,
       2,
       3
    )
    

    works in JavaScript, so it’s probably something else.



  • @RaceProUK said:

    Depends on the language; C# has had optional method parameters from version 4, and they come in handy occasionally.

    It looks like JavaScript, and JS has had optional params forever. But apparently it's not...

    Anyway, I agree this is boilerplate for the sake of boilerplate, definitely delete.



  • @Eldelshell said:

    some limitation of the PL we're using

    What's a PL? Google brings up a kajillion things, none of which seem relevant.


  • sockdevs

    @blakeyrat said:

    What's a PL? Google brings up a kajillion things, none of which seem relevant.

    Call me crazy, but it might just stand for 'programming language'...


  • Discourse touched me in a no-no place

    Quite possibly Programming Language. Probably an AFTSOAA...



  • Why the fuck would anybody abbreviate "programming language" to PL? Especially when PL is perl's file format.

    If that's a correct guess, I am SO pissed. Every problem on this forum is due to you people not being able to communicate worth shit.


  • sockdevs

    @blakeyrat said:

    Why the fuck would anybody abbreviate "programming language" to PL?

    Because... racecar?



  • Search for OOPL.

    About which PL™ this is, it's called BrightScript™ and is used by Roku™



  • @Eldelshell said:

    Search for OOPL.

    You didn't type OOPL. If you had, I would have Googled "OOPL" and understood what the hell you were talking about.

    @Eldelshell said:

    About which PL™ this is, it's called BrightScript™ and is used by Roku™

    Why didn't you just put that in the OP instead of 76 people having to squeeze it out of you?



  • It's not relevant to the discussion since this pattern is reproducible in any PL™.



  • They might be abusing a pattern that jQuery uses a lot.

    function Foo(options) {
      var defaults = {style: "normal", position: "center"}
      options = $.extend(defaults, options);
      // do stuff
    }
    
    
    Foo({style: "bold"});
    

    It's a useful pattern if there are a lot of parameters and nearly all of them are optional.



  • E.g.

    PL/1, Programming Language / One

    <waves cane/>


    Which also interfaces well with my zoned-decimal rocks



  • @Jaime said:

    It's a useful pattern if there are a lot of parameters and nearly all of them are optional.

    ... and your language doesn’t support named optional parameters.



  • Anti-pattern? This looks a lot like the Builder Pattern to me

    Well, let me rephrase that: Passing the items in as a single object is the Builder pattern. In turn passing them to another function individually is probably some sort of anti-pattern.



  • A better "pattern" is to use a proper language that supports optional named parameters.


  • I survived the hour long Uno hand

    Fuck jQuery. They do shit like that and everyone starts doing it because "jQuery does it so it must be right".



  • @Yamikuronue said:

    They do shit like that and everyone starts doing it because "jQuery does it so it must be right".

    That's called "Cargo Cult Programming". The problem here isn't jQuery, it's the people who copy it without considering what they are copying.



  • @Eldelshell said:

    Am I TRWTF?

    I kinda like the first form, especially when there are a lot of arguments. x, y, z is not a good example, but when you have f(numberOfThings, arrayOfThings, andAnotherThing, andSomethingElse, andSomethingForInitialization, andABooleanJustBecause, andLetsThrowInAKitchenSink), I think it's better to wrap it in some object.



  • I don't know, I like this pattern. Like Jaime said: @Jaime said:

    It's a useful pattern if there are a lot of parameters and nearly all of them are optional.

    But I just wish JavaScript would get keyword arguments like Python:

    def print_args(req_arg1, opt_arg2 = "Default arg 2", opt_arg3 = "Default arg 3"):
        print req_arg1
        print opt_arg2
        print opt_arg3
     
    
    print_args("Actual  val 1", opt_arg3 = "Actual  val 3")
    # Actual  val 1
    # Default val 2
    # Actual  val 3
    


  • Well it kind of is because that might be necessary in the language. Maybe it's a crap language that shits itself if you have more than 3 params or something, who knows.

    But whatever, the real point is PL is not an established acronym for anything so you confuse the bejeesus out of everybody when you use it. And the explanation far outweighs the 28 milliseconds of typing you saved.



  • That's nothing. You, like me, like .net. We have to deal with two drastically different meanings of "CLI" though not, perhaps, on a regular basis


  • sockdevs

    @blakeyrat said:

    But whatever, the real point is PL is not an established acronym for anything so you confuse the bejeesus out of me when you use it.

    FTFY :stuck_out_tongue:

    The meaning was pretty clear to everyone else from the context.



  • @Magus said:

    We have to deal with two drastically different meanings of "CLI"

    I have no idea what you are talking about. Unless you use "CLI" to mean "CLR"?

    @RaceProUK said:

    The meaning was pretty clear to everyone else from the context.

    And yet, since he still had to explain his idiotic acronym he typed for no good reason except to confuse people, there was no net savings of time or effort for anybody involved. And a lot of extra confusion.



  • @blakeyrat said:

    I have no idea what you are talking about. Unless you use "CLI" to mean "CLR"?

    No, it's more abstract than that. They mentioned it quite a lot in the C# language specification iirc. The "Common Language Interface" is, as far as I know, an ideal implemented as CIL, just to make it worse.



  • @blakeyrat said:

    I have no idea what you are talking about. Unless you use "CLI" to mean "CLR"?



  • @Magus said:

    The "Common Language Interface"

    I'm pretty sure I've never needed to say that in the entire time I've been writing C#.



  • I've only heard it in the context of C++/CLI which is like C++ .NET or something.



  • That just means you didn't read the specification. Boring stuff, but good. Also, @boomzilla pointed out my failure to remember it right.



  • @blakeyrat said:

    I have no idea what you are talking about. Unless you use "CLI" to mean "CLR"?

    I haven't write any C# in 8 years and I knew what he was talking about. Then again I was using Mono so maybe that's why I knew what the CLI was and why it's important.

    @blakeyrat said:

    he still had to explain

    I did not explain it.


Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.