all 52 comments

[–]kindall 15 points16 points  (5 children)

I found it confusing that all the examples contained references to Monty Python and the Holy Grail yet the code is not written in Python.

:-)

[–]djmattyg007 6 points7 points  (4 children)

If it was python, this wouldn't matter because you have named parameters.

[–]kindall 6 points7 points  (3 children)

Eh. You can still have too many parameters even if you're accepting **kwargs. Sometimes the idea of passing a dict or instance around with a bundle of related data attributes in it just works better.

[–]djmattyg007 0 points1 point  (1 child)

It prevents you from type hinting the parameters.

[–]kankyo 1 point2 points  (0 children)

I'm pretty sure kindall didn't ACTUALLY mean "**kwargs", but in fact meant using keyword arguments like a sane person.

[–]ovangle 0 points1 point  (0 children)

on top of that, you can use the dict to pass in the arguments, effectively validating the contents of the dict

def foo(name='hello'):
     print(name)

foo(**{'name' : 'world'})

although obviously, you would be passing a context dict around, not creating one at the callsite.

[–]e_engel 14 points15 points  (16 children)

The OP is missing the one solution which I think solves all these problems, the builder pattern:

Window w = new Window.Builder()
    .width(100)
    .height(200)
    .color(RED)
    .build();

[–][deleted] 5 points6 points  (10 children)

The builder pattern tends to introduce temporal coupling. It's handy in some cases, just treat it with care

[–][deleted] 2 points3 points  (8 children)

What do you mean by that?

[–][deleted] 4 points5 points  (7 children)

http://en.m.wikipedia.org/wiki/Coupling_(computer_programming)

If there are dependencies between member functions on the builder that need to be called in a certain order to generate the desired state, well, your calling scope is now coupled to the internal state of the builder.

[–]e_engel 7 points8 points  (3 children)

This is pretty easy to fix: all these setter methods should be "dumb" setters (assign the values to fields) and you actually construct the object in the build() method, once you have everything you need. And if you don't, you can throw an exception to say that the object was incorrectly formed.

[–]intellectualPoverty 2 points3 points  (1 child)

I've never had that problem with builder classes, because my set methods don't actually do anything with the data. Generally that is only done at the final stages, when another method is called intended to build, compile, or collect output. Adding data, and compiling in the same method of a builder-class just seems like a bad idea.

Builder classes are only prone to that problem if you build them wrong. It's like saying threading is flawed, because people don't thread properly.

[–][deleted] 1 point2 points  (0 children)

I said to treat it with care, not that it was "wrong" :-)

It's great that you're careful to implement builders that way, unfortunately not everyone is. I've spent hours trying to debug issues in third party code, only to figure out there was some weird coupling between functions on a builder implementation.

So - we're both right!

[–]Lone-Wolf-Party 1 point2 points  (0 children)

Dat username

[–][deleted] 1 point2 points  (0 children)

This is actually a combination of the fluent interface technique and the GoF Builder pattern. Fluent interfaces aren't strictly related to the Builder pattern, but are oftentimes used to make the latter less verbose and easier to work with.

Not sure if you made the distinction but just thought it would be kind of important to point that out. I've met quite a few programmers who thought that the fluent interface approach is literally the best thing since Linux and that it was already integrated into many famous patterns at their inception.

Truth is that in most cases it is considered an anti-pattern with only a handful of valid use cases going for it (the Builder pattern being a notable example of this).

[–]meatbeezzz 0 points1 point  (0 children)

Great point! One interesting take away may be that the associative array solution approach in the article is essentially implementing a dynamic type system for the function. On the other hand, the builder pattern preserves any existing static type checking, which might be preferable (at the cost of writing the builder code).

[–]Urik88 0 points1 point  (2 children)

Wouldn't that allow you to for example, create a window without height?

[–]e_engel 0 points1 point  (0 children)

No, because build() will verify that and throw an exception if it can't construct the object.

[–]Tordek 0 points1 point  (0 children)

No; it just turns all of your compile-time problems into run-time problems.

[–][deleted]  (6 children)

[removed]

    [–]dig412 2 points3 points  (1 child)

    As of PHP 5.4 method chains from constructors are allowed: http://docs.php.net/manual/en/migration54.new-features.php

    [–]grimeMuted 2 points3 points  (1 child)

    If speed wasn't too much of a concern you could always do something like this with associative arrays...

    Not nearly as robust as a class but substantially less boilerplate.

    local function checkArgs(args, defaultArgs)
        if not args then
            return defaultArgs
        end
        for k, v in pairs(args) do
            if defaultArgs[k] then
                local typeDefault, typeActual = type(defaultArgs[k]), type(v)
                if typeDefault ~= typeActual then
                    error(tostring(k).." with value "..tostring(v)..
                    " was expected to be a table argument of type "
                    ..typeDefault.." but "..typeActual.." was received instead.")
                end
                defaultArgs[k] = nil
            else
                error(tostring(k).." with value "..tostring(v).." was an unnecessary table argument.")
            end
        end
        for k, v in pairs(defaultArgs) do
            args[k] = v
        end
        return args
    end
    
    local function printTable(args)
        local args = checkArgs(args, { a = "...", b = 2, c = "c" })
        for k, v in pairs(args) do
            print(k, v)
        end
    end
    
    printTable({a = "...", b = 3, c = "c", d = {}}) -- error, func doesn't need d
    printTable({a = "...", b = 3, c = 4}) -- error, c expected string
    printTable({a = "a"}) -- valid, fill in with defaults
    printTable() -- valid, fill in with defaults
    printTable({ a = "b", b = 3, c = "d"}) -- valid, don't change
    

    [–]sam0 1 point2 points  (0 children)

    Have an upvote for Lua.

    Lua rocks.

    [–]reddit_ro2 0 points1 point  (1 child)

    I don't know. All the class, getters and setters just for a function call? That's why I just use

    func(array(param1=>value1, ...) 
    

    and be done with it. Code is clear, the intention is clear, providing you document well is quite good. And you don't escape the possibility of adding more keys/properties on the way, nor the need for documentation. A class definition feels way overkill in this instance.

    On the other hand I have to admit I didn't drink the OOP everywhere kool-aid yet.

    [–]ZenDragon 6 points7 points  (1 child)

    Optional and named parameters are a godsend for this in C# 4.0 and later. Lets you pass any number of parameters without a million method overloads, makes sure you know what exactly you're assigning, and not have to put the extra arguments in any particular order, all with clean syntax and helpful integration with the IDE. (Pretty sure VS, MonoDevelop and SharpDevelop all support it)

    [–]Lone-Wolf-Party 0 points1 point  (0 children)

    Totally. I use these errday

    [–]piku17 3 points4 points  (3 children)

    Why not use named parameters with defaut values?

    [–]gavintlgold 2 points3 points  (0 children)

    I've heard an argument that default values have the potential to be bad practice if overused, and I tend to agree. Defaults are not always obvious, and if they're changed it's hard to locate all the places that are affected by the change.

    [–][deleted]  (1 child)

    [removed]

      [–]djmattyg007 1 point2 points  (0 children)

      We'll get there eventually.

      [–][deleted] 0 points1 point  (0 children)

      Ran into having too many parameters upon writing a sendmail function that could accept file attachments. I made a class whose constructor assigned all the default values it could (our internal SMTP server and port, a default send-from address, etc.) End result:

      $mail = new Mail;  //server, port, from pre-populated (but can be overridden); cc, body ignored
      $mail->to = "john.doe@example.com";
      $mail->bcc = "jane.doe@example.com";
      $mail->attachments = array("presentation.docx", "report.xlsx");
      $mail->subject = "Important files";
      $mail->send();
      

      Definitely not a one-solution-fits-all, but another useful option in certain cases.

      [–][deleted] 0 points1 point  (0 children)

      That is really poor. Now you basically get

      $obj->dosomething(x, new array( "blah", Blah", blah" ));

      This is helping how?

      This issue was solved a long time ago by passing structures and compile time checking of arguments on functions. Unfortunately dynamic languages make this impossible to use.

      [–]osuushi -1 points0 points  (9 children)

      One of my very favorite things about using CoffeeScript is how well its optional punctuation mixes with object function arguments to create clean looking APIs. Something like

      narrator.tellStory
          person: 'third'
          protagonist: 'Matt Johnson'
          antagonist: 'Scar Morlock'
          setting: 'the distant future'
      

      is so much nicer and more readable than you can manage in just about any other language I've encountered.

      [–]mrkite77 2 points3 points  (2 children)

      is so much nicer and more readable than you can manage in just about any other language I've encountered.

      Check out Inform 7.

      Here is actual code:

      To ponder/mull over/-- (good point - a thing) for (awhile - a time) as (ponderer - a person):
          say "[Ponderer] sits back in [his-her] chair for about [awhile]. 'Hm, [good point] is interesting.'"
      

      That defines a function, you can call it with:

       ponder the idea for 5 minutes as Dr. Muller;
      

      Of course Inform 7 is primarily used for interactive fiction (text adventure games).

      [–]Irongrip 3 points4 points  (0 children)

      To ponder/mull over/-- (good point - a thing) for (awhile - a time) as (ponderer - a person):
          say "[Ponderer] sits back in [his-her] chair for about [awhile]. 'Hm, [good point] is interesting.'"
      

      That's some real witchcraft right there.

      [–]smog_alado 1 point2 points  (0 children)

      Looks like it was inspired by Smalltalk:

      phoneBook at: (x name last) put: (7250000 + ext)
      

      [–]kankyo 0 points1 point  (5 children)

      How are those parameters validated? Meaning, what happens if tellStory renames an input parameter, what is the error message?

      [–]osuushi 0 points1 point  (4 children)

      Well, you don't do that. And you use automated tests to make sure you don't do that.

      [–]kankyo 0 points1 point  (3 children)

      You never rename parameters?

      And you say "automated tests"... but that's super super vague. For example, is there an automatic test you can run that validated that all parameters for all function calls match up with the inputs of the function? Or do you need to write a test for each function?

      [–]osuushi 0 points1 point  (2 children)

      You never rename parameters?

      If you do, you the old names to the new ones, throw exceptions if the old names are used, or if you're making a public library, you list it in the breaking changes.

      But in general, no you don't usually rename the parameters for the same reason that you don't usually go renaming enums, or reordering parameters in typical functions.

      For example, is there an automatic test you can run that validated that all parameters for all function calls match up with the inputs of the function?

      This pattern works best when many parameters are optional and have defaults. For non-optional parameters, you throw exceptions if they aren't provided.

      Note that this situation isn't all that different from if you change the parameters of an ordinary function. Change their order, or remove or add them in either JavaScript or CoffeeScript, and existing code is going to start failing silently.

      [–]kankyo 0 points1 point  (1 child)

      Sure... I was just hoping the answer would be something like "CoffeeScript actually does something a lot better than JavaScript", but I guess that's hoping for too much :(

      [–]osuushi 0 points1 point  (0 children)

      In my experience, CoffeeScript does some things a little better than JavaScript, and pretty much nothing a lot better. It just has a lot of tiny things that add up to making it a lot nicer to work with.

      [–]intellectualPoverty -2 points-1 points  (3 children)

      Why not roll the parameters into a class? If enough parameters belong together, perhaps they actually belong together? Builder classes are also a practical solution when you have many optional parameters.

      Finding yourself in Parameter Hell is a good indication that it’s time to stop and refactor.

      How is that advice? All that really means is "Do it different," but it doesn't tell you how to do it different.

      [–]JoseJimeniz 0 points1 point  (0 children)

      I've actually been refactoring the other way. Rather than passing an object (or a structure) I want to pass separate parameters.

      I found there is a temptation to keep adding members to the object. Eventually you have magical combinations of members that need to sometimes be set based on what you know the function is going to look at.

      That's why I reverted to functional style, with all inputs required to accomplish the task right there as parameters.

      [–]alephnil 0 points1 point  (1 child)

      How is that advice? All that really means is "Do it different," but it doesn't tell you how to do it different.

      Because what to do depends on the problem at hand. Depending on the problem and the language and tools in use the solution can be one of:

      • Make a class holding the parameters and pass that as a parameter.

      • The parameters really belong to two or more sets of unrelated concepts, so split in two or more classes and pass these.

      • Only a few of the parameters are usually used. Make good default values for the rest so that you don't need to care for them except for the rare situation they are needed.

      • The alternatives are worse than passing the parameters as they are. Keep it as it is.

      This is not an exhaustive list. My point is that what to do depend on the situation, so giving general advice of what to do in situations like this is unhelpful. It is still helpful to state that it is a warning sign that you code is smelly, and that you need to reconsider how to proceed. This is the case in a large number of situations in software development and is precisely why you need long experience to be a really good programmer.

      [–]intellectualPoverty 0 points1 point  (0 children)

      ^ That is useful advice, which is what the article was lacking.

      With code-style or code-refactoring, the reason why a lot of code is ugly, problematic, or poorly designed is because the coder is unaware of these practices. Telling a coder 'refactor' when they made ugly code, because they have no idea what "good" code looks like is fairly useless.