Interesting way to pass parameters...



  • This morning I had the misfortune of reviewing some of a coworker's code.  I was adding a test to our test suite, and I noticed that about half of the tests he wrote were false positives, and have no effect other than to make the build take twice as long.  But that's not the WTF.

    Inside this file, I was looking at his data setup utility method, since I would obviously want to use them for my own test.  And in it, I find something quite curiuos.  The test (which is contained in an instance method with the [TestMethod] attribute (we're using .NET 4.0)) sets one or more static variables (one of which is, for example, a bool called PreferredCust).  The test then calls a static data creation method, which has no side effects, but simply builds an object used as the test data.  In one of the nested methods, a few frames up the call stack, the static bool is used.

    Here's a simplified analogy:

    public class MyTest{

     private static string CustomerName { get; set; }

    [TestMethod]

    public void RunTest(){

    CustomerName = "Bob";

     BuildCustomer();

    }

    public static Customer BuildCustomer(){

    return new Customer{

    Name = CustomerName

    };

    }

    }

    You are probably asking why this bool is not simply passed through the methods, and included in the method signature.  If we were using .NET <4.0, one might suggest that it's simply too annoying to create countless overloads of the same method, just to set a default value.  That would simply be an appeal to laziness.  Of course, .NET 4 has default parameters, so all I can conclude now is incompetence.

    It's a really good thing these tests run one at a time.



  • @ShatteredArm said:

    You are probably asking why this bool is not simply passed through the methods, and included in the method signature.  If we were using .NET <4.0, one might suggest that it's simply too annoying to create countless overloads of the same method, just to set a default value.  That would simply be an appeal to laziness.  Of course, .NET 4 has default parameters, so all I can conclude now is incompetence.

    It's a really good thing these tests run one at a time.

    That's what you have IDE's for... This type of thing was an antipattern even back in the C days.



  • @dtech said:

    That's what you have IDE's for... This type of thing was an antipattern even back in the C days.
     

    explain. because even if IDEs have some macro (which i don't know about, and would be happy to learn about) to generate the overload functions, it would still produce the  same result in code as writing them. having to (write || maintain) 26 constructor overloads is pain.

    and what's worse, sometimes you can't find other way to do things.
    (Wrapping a class that has 16 constructor overloads to extend it, adding one optional parameter to the "parent" class, and doing some pre-processing before sending the data to the "parent" object functions.) 



  • @SEMI-HYBRID code said:

    having to (write || maintain) 26 constructor overloads is pain.

    Ouch!

    @SEMI-HYBRID code said:

    any tips?

    Delete 21 constructors?

     



  • Tips?
    public class MyClass
    {
       public MyClass(int thingy = 0, string frobby = null, string foobar = "FizzBuzz")
    }
    With named parameters, that right there is 8 overloads, I'm sure you could see more. Here's another way of doing the same.
    public class MyClass
    {
       public MyClass(int thingy, string frobby, string foobar) {};
       public MyClass(int thingy, string frobby) : this(thingy, frobby, "FizzBizz") {}
       public MyClass(int thingy) : this(thingy, null) {}
       public MyClass() : this(0) {}
    }
    Still only have one function to actually maintain
     
    edit: Deleted code tags because it looked like crap.  How do you make it look good?


  • ♿ (Parody)

    @Sutherlands said:

    Deleted code tags because it looked like crap.  How do you make it look good?

    You want
      the <pre>
                 tags
    


  • @fatbull said:

    @SEMI-HYBRID code said:

    having to (write || maintain) 26 constructor overloads is pain.

    Ouch!

    @SEMI-HYBRID code said:

    any tips?

    Delete 21 constructors?

     

    Delete tham all and use [url="http://msdn.microsoft.com/en-us/library/bb384062.aspx"]initializers[/url].  Ideally, constructors should be used to guarantee that an instance is created in a valid state, not as syntactical sugar to avoid setting properties.


  • @Jaime said:

    Delete tham all and use initializers.  Ideally, constructors should be used to guarantee that an instance is created in a valid state, not as syntactical sugar to avoid setting properties.
    +1


  • 26 constructor overloads? With constructors that have up to 20 or more parameters? You should really consider using the builder pattern. Then you still need a (package or assembly access) constructor, but only one, which is called by your builder.

    new FooBuilder().withBar(24).withStuffDisabled().forCustomer("Bob")
        .withMaxPrice(20, Currencies.USD).build();
    

    Edit: Initializers are fine, but bad if you want/have to provide an API where it is important that your objects are immutable after creation.



  • @mihi said:

    26 constructor overloads? With constructors that have up to 20 or more parameters? You should really consider using the builder pattern. Then you still need a (package or assembly access) constructor, but only one, which is called by your builder.
    new FooBuilder().withBar(24).withStuffDisabled().forCustomer("Bob")
        .withMaxPrice(20, Currencies.USD).build();
    

    Edit: Initializers are fine, but bad if you want/have to provide an API where it is important that your objects are immutable after creation.

    Obviously initializers don't work if your object is immutable... but in that case you would only have one constructor anyway...


  • @SEMI-HYBRID code said:

    because even if IDEs have some macro (which i don't know about, and would be happy to learn about) to generate the overload functions, it would still produce the  same result in code as writing them. having to (write || maintain) 26 constructor overloads is pain.

    If you have 26 overloads I think it's a better idea to create a small factory than to put all of the logic in the constructor. In facts if you want to have 26 different ways to call a function you're doing it wrong imho. As for the IDE example: out of the top of my head I could only think of the Eclipse Refactor->Change Method Signature options, but i'm 99% sure that it has a better way of generating overloads to emulate default parameters.

    Of course default parameters are 10x easier and make a lot of sense, but passing your parameters through static or global variables is never the right option if you don't have them.



  • @mihi said:

    26 constructor overloads? With constructors that have up to 20 or more parameters? You should really consider using the builder pattern. Then you still need a (package or assembly access) constructor, but only one, which is called by your builder.

     

    new FooBuilder().withBar(24).withStuffDisabled().forCustomer("Bob")
        .withMaxPrice(20, Currencies.USD).build();

     

    When my functions or "classes" in javascript have more than 3 arguments for calling/constructing, I always upgrade the argument list to an object literal.

    This is essentially the same as C# initializers (after a cursory glance), but with the added benefit of that builder pattern: you don't need to make all your properties public.

    At any rate, we agree that calls like this:

    foobar(12, "Bob", true, false, MAX_COUNT, "Megatron")

    are poorly readable and unnecessary. :)

     



  • @dhromed said:

    @mihi said:

    26 constructor overloads? With constructors that have up to 20 or more parameters? You should really consider using the builder pattern. Then you still need a (package or assembly access) constructor, but only one, which is called by your builder.

     

    new FooBuilder().withBar(24).withStuffDisabled().forCustomer("Bob")
        .withMaxPrice(20, Currencies.USD).build();

     

    When my functions or "classes" in javascript have more than 3 arguments for calling/constructing, I always upgrade the argument list to an object literal.

    This is essentially the same as C# initializers (after a cursory glance), but with the added benefit of that builder pattern: you don't need to make all your properties public.

    At any rate, we agree that calls like this:

    foobar(12, "Bob", true, false, MAX_COUNT, "Megatron")

    are poorly readable and unnecessary. :)

    I somewhat agree with you, however using an object literal allows the consumer to instantiate your class with an invalid state.  At least with ctor overloads (and I agree, there should be a limit on parameters and overloads), you know that you're forcing/steering the consumer to create a valid instance.  Also with object literals, there is nothing implicit that guides the developer on how to create an instance with a valid state.  Using overloads, you at least have the method signatures to guide you.



  • @dhromed said:

    At any rate, we agree that calls like this:

    foobar(12, "Bob", true, false, MAX_COUNT, "Megatron")

    are poorly readable and unnecessary. :)

    Yes. Obviously he should have gone with Starscream in that case.



  • @dhromed said:

    At any rate, we agree that calls like this:

    foobar(12, "Bob", true, false, MAX_COUNT, "Megatron")

    are poorly readable and unnecessary. :)

    I hate to mention it, but if you truely think this is unreadable, then I have a solution for you; Visual Basic, circa 1995.  VB has had [url="http://en.wikipedia.org/wiki/Named_parameter"]named arguments[/url] for all function calls for a really long time.  Any problem whose solution is "use a really old version of Visual Basic" probably isn't a problem at all.



  • @Jaime said:

    I hate to mention it, but if you truely think this is unreadable, then I have a solution for you; Visual Basic, circa 1995.  VB has had named arguments for all function calls for a really long time.  Any problem whose solution is "use a really old version of Visual Basic" probably isn't a problem at all.
     

    You're either joking, or beyond redemption.


  • @dhromed said:

    @Jaime said:

    I hate to mention it, but if you truely think this is unreadable, then I have a solution for you; Visual Basic, circa 1995.  VB has had named arguments for all function calls for a really long time.  Any problem whose solution is "use a really old version of Visual Basic" probably isn't a problem at all.
     

    You're either joking, or beyond redemption.

    Not mutually exclusive


Log in to reply