Rule Of Thumb: Always Define And Consume An ArgumentCollection Map In A Single Context
ColdFusion has the awesome ability to programmatically define function arguments before a given function is invoked. Depending on how this gets used, this can be awesome; or, it can be a frustrating debugging nightmare for you or the next developer. To make sure you keep it awesome, follow this rule of thumb: Always define and consume the "argumentCollection" in a single context.
If you're unfamiliar with the argumentCollection syntax, it's basically a way to map a struct onto a set of named or ordered function arguments:
<cfscript>
// Define the method arguments as a collection. This allows us to conditionally
// build the arguments before invoking the target function.
args = {};
args.a = "Foo";
args.b = "Bar";
args.c = "Baz";
// Invoke the method using the above collection as an arguments map.
doSomething( argumentCollection = args );
// -- Methods. -- //
function doSomething(
required string a,
required string b,
required string c
) {
writeDump( arguments );
}
</cfscript>
As you can see, I'm defining my collection of argument values before I actually invoke the function, doSomething(). Then, when I invoke the function, I tell ColdFusion to use my struct as the source of arguments. It's a very powerful feature.
Ok, now that you understand the argumentCollection, let's take a look at an anti-pattern - defining the argumentCollection and consuming it in two different contexts:
<cfscript>
// Define a set of values that we want to pass to our first function. This is NOT
// an arguments-map, this is a single argument. If you are working with a Framework,
// you can think of this like the "RC" (Request Collection) object (as an example).
rc = {};
rc.a = "Foo";
rc.b = "Bar";
rc.c = "Baz";
// Pass the inputs to the first function.
doSomething( rc );
// -- Methods. -- //
function doSomething( required struct rc ) {
// This method does not directly consume the inputs. It augmenst the inputs and
// passes them through to another function.
// --
// CAUTION: This is (in my opinion) a huge ANTI-PATTERN. It is not self-
// documenting in any way, and it requires the developer to build a mental model
// of the inputs that crosses multiple function calls. This is far too much
// overhead and causes loads of frustration. And, it allows for unexpected values
// to be passed-through simply by being part of the original input collection.
rc.d = "Dang";
doSomethingElse( argumentCollection = rc );
}
function doSomethingElse(
required string a,
required string b,
required string c,
required string d
) {
writeDump( arguments );
}
</cfscript>
As you can see, we're defining our arguments collection - rc - in one context but, we're consuming it (as argumentCollection) in another context. This is extremely problematic for the several reasons:
It is not self-documenting. Since the incoming value-set can contain anything, there's no way to know, in the context of doSomething(), what will truly exist in the inputs.
Because the argumentCollection is defined in one context and consumed in another, it requires the developer to build an unnecessary mental model that spans function calls.
It allows "argument cruft" to pass through to the second function invocation. Since there is no contract on what can be in the struct, changes to the "rc" can have an unexpected impact on how doSomethingElse() functions.
It changes the state of the original "rc" struct, which was passed into the first function by-reference. This may or may not cause unexpected behavior later on in the page's control flow.
The better approach is to create an intermediary argumentCollection that clearly and explicitly maps the incoming values onto the next set of function arguments:
<cfscript>
// Define a set of values that we want to pass to our first function. This is NOT
// an arguments-map, this is a single argument. If you are working with a Framework,
// you can think of this like the "RC" (Request Collection) object (as an example).
rc = {};
rc.a = "Foo";
rc.b = "Bar";
rc.c = "Baz";
// Pass the inputs to the first function.
doSomething( rc );
// -- Methods. -- //
function doSomething( required struct rc ) {
// BEST PRACTICE: Instead of augmenting the "rc" and passing it through to the
// next function, define a new argument-map that will be used to invoke the next
// function. This provides a very clear contract for the argument collection, is
// self-documenting, doesn't let "argument cruft" flow through, and requires no
// mental gymnastics on the part of the developer.
var inputs = {};
inputs.a = rc.a;
inputs.b = rc.b;
inputs.c = rc.c;
inputs.d = "Dang";
doSomethingElse( argumentCollection = inputs );
}
function doSomethingElse(
required string a,
required string b,
required string c,
required string d
) {
writeDump( arguments );
}
</cfscript>
As you can see here, the "rc" object is mapped onto an intermediary struct that is used to invoke the next function. This is self-documenting, intuitive, and creates a much more pleasurable debugging experience (where we spend most of our time as developers).
Want to use code from this post? Check out the license.
Reader Comments
If you are going to do the intermediate step, you can clean it up a little:
var inputs = {
a = rc.a,
b = rc.b,
c = rc.c,
d = "Dang"
};
fwiw, I go back and forth between the different sides of this argument. I like being declarative and self-documenting, but I also like to pass an rc struct to multiple service methods, augmenting and building along the way. I end up doing exactly what you have done here a lot of the time though.
Nope, this isn't an argumentcollection issue you're demonstrating - the real problem is that you're passing in a generic struct as a single argument.
Beyond that, it's not really possible to comment on a better approach because you've used non-semantic names and the best approach depends on the individual situation - sometimes it's creating an intermediary struct, sometimes it's clone & modify, sometimes it's something else - there isn't a one-size-fits-all rule.
@Peter,
I didn't mean to imply that it was a problem with argumentCollection. In fact, I really like argumentCollection and I think it's a very strong feature of the language. My only concern was using an incoming struct (defined out of context) as the argumentCollection in the current context.
And, I agree that it's probably not one-size fits all. But, I would guess that in most of the breaking-the-rule scenarios, the various augmenters of the struct would be part of a cohesive piece of functionality:
var args = {};
this.augmentWithSomeStuff( args );
this.augmentWithOtherStuff( args );
useArgs( argumentCollection = args );
But, probably at _no_ time should a generic FORM / URL scope be used as the argumentCollection as this provides no documentation and not only spans server-side context, but client-side context as well.
@Ryan,
Agreed - that's the syntax I usually use. I forget why I defined the {} first and then the keys. I think I was probably going to add some additional comments (before a particular key); but, then ended up not doing that.
> But, probably at _no_ time should a generic FORM / URL scope be used as the argumentCollection
Except for the times when they should. ;)
An example might be wrapping a function from a REST api - if it usually takes its arguments from the url scope but is being called manually, calling doSomething(argumentcollection=url) or doSomething(override=something,argumentcollection=url) is simple, obvious, and avoids the brittleness that specifying each of the keys would bring.
@Peter,
Hmmm. I'll have to noodle on that one a bit more. That said, I wouldn't consider it brittle if changing the inputs breaks the workflow. I find a certain degree of security in that. The trade-off being that I have to make the update in 3 places (client, controller, service) instead of just 2 (client, service).
Perhaps we can chalk this up to coding style, as opposed to best-practices.
I didn't reply to this last night when I first read it as I'd had a coupla beers. In the meantime @Peter's come along and said exactly what I thought last night, and what I still think today, with a clear head.
The underlying premise of this article is faulty, so the logic that follows is misguided.
The problem here is that your misappropriating your request context object in your second method call. The first method expects ONE argument: a request context (rc). Fair enough. Then you're passing the rc as a set of individual args, which really doesn't make a lot of sense to do, generally speaking. There would be a case for doing it, I'm sure, but it's impossible to say here due to you using too-neutral-to-be-useful method names.
So your making a case from an artificially contrived situation which doesn't really make sense.
What you're really demonstrating is that it would seldom be correct to have an object (rc) and then have code that expects its individual components as separate arguments. That's just poor coding.
Your follow-up comment to Peter - which he again says exactly what I would have said - is also faulty.
You're trying to invent a rule of thumb here for a case where no rule of thumb should exist.
argumentcollection is simply a convenience mechanism. As with *any* code, one should assess whether it's being used the correct way for the given situation, and "good code" should trump "convenience" sometimes. But often "good code" and "convenience" will overlap. There is no generic "rule of thumb" that sensibly covers how one should use it.
Sorry mate.
--
Adam
@Adam,
It sounds like you're saying, this rule doesn't make any sense _if_ you're writing good code, because good code would preclude the use of such a rule. But, that's what I'm saying - use this rule to write good code. You're just assuming that someone is going to write good code in the first place, which I am not.
As far as the "rc" thing, I am not sure I follow. Maybe this just comes down to coding style? In many frameworks, the "Rc" is the combination of URL + FORM scopes. So, wouldn't it make sense that you _often_ have controller methods that accept the single rc object, and then have to turn around and invoke the app-core to perform logic. Maybe a more concrete (pseudo) example:
function submit( rc ) {
param name="rc.name" type="string";
param name="rc.email" type="string";
param name="rc.password" type="string";
userService.createAccount( rc.name, rc.email, rc.password );
}
In this case, the "Rc" is translated into individual arguments, instead of being passed-through.
@Adam,
Unless, you're just saying that re-creating of an argumentCollection is silly in my case - as in, why create the argumentCollection and *not* just invoke the method with individual arguments (as in my previous comment)? In which case, I would agree with you.
The thing is I don't think anything you say here has any bearing on whether the code is good or not, and what your advocating will neither improve nor degrade code quality.
Basically what you're describing as an anti-pattern *isn't* one, and you build your case as that as your foundation, which - IMO - invalidates what you're saying.
Perhaps with a less context-neutral example it'd be easier to see whether what you're suggesting has merit. But in that I think what you're saying in your context-neutral example is *wrong*, I think this indicates it's not a "rule of thumb".
That said... you know what they say about opinions, eh?
--
Adam
@Adam,
I honestly do think that we are on the same page. You say:
>> What you're really demonstrating is that it would seldom be correct to have an object (rc) and then have code that expects its individual components as separate arguments. That's just poor coding.
Exactly. This is what I *really* see in code. This isn't a made-up example. In fact, this is the example that got me thinking about the "rule" in general. Rc is being used as an argumentCollection in one context, but is defined in a different context (ie, by the framework). And, I'm saying, Don't do that... which it sounds like you agree with.
I think maybe the disconnect is that you don't believe that that is a "real" example?