ColdFusion: a piece of functionality should do one thing, and do it well

G'day:
One of my pet hates is having a function called doX(), and doX() does X, but it also does Y.

An example of this is CFML's isValid() function which not only checks validity - as one might expect - but also trims any string values its validating first. The function is not called isValidAfterTrim(), so it should not do the trimming. If one wants to trim a value before validating it... call trim() then isValid(). Simple. Another example is <cfdump> which picked up and abort attribute along the way. This was because some mungbean said "well usually I want to dump and abort, so why can't dump just abort too". And for some stupid reason Adobe went "oh yeah... let's do that". No. If you want to abort after you dump... do an abort after your dump. Don't change <cfdump> to be <cfdumpandabortifyoulike>. That's dumb. At least the Lucee ER to have <cfinclude> also do an abort got rejected ("Add optional abort Attribute to cfinclude"). FFS.

The reason to this is to keep units of functionality small, for starters. It makes them easier and more coherent to test. Also the more functionality one piles into a function (and for this purpose, a tag is just a function that looks funny ;-), the more potential for it being broken as it's maintained. Plus it makes code less clear and less clean.

A function should do one thing, and do it well.

Anyway, today I got wind of this ER: "encodeFor attribute for cfoutput, writeOutput". The intent is honourable, but it suffers from trying to do too much. The gist of it is:

While ColdFusion 10 added the various ESAPI encodeFor* functions, it is dependent upon the developer to properly wrap location where used with the appropriate function (e.g. <cfoutput>#EncodeForHTML(url.name)#</cfoutput> ). Adding an attribute encodeFor negates the need for wrapping individual variables and would process the entire block contained within <cfoutput>cfoutput>cfoutput> for anything within #'s with the appropriate ESAPI EncodeFor* function specified.

On the face of it this sounds reasonable. It still smacks of "doing too much", so I echoed my position as much.

I sometimes doubt I'm being too dogmatic about these things, so I asked on the #CFML Slack channel, and John Whish came up with a great counter example:

<cfoutput encodeFor="HTML">
    #url.name#
    <a href="whatever.cfm?id=#url.id#" onclick="get(#url.id#)">Go to whatever.cfm?id=#url.id#</a>
</cfoutput>

What's this gonna do? Encode all the expressions within it for HTML. Which is only correct for #url.name#. #url.id# should be encoded for URL, JavaScript and URL for each of its three respective uses. However a less than discerning dev might not "get" that, and now their output is encoded incorrectly.

So what we'd need to then do is this:

<cfoutput encodeFor="HTML">#url.name#</cfoutput>
    <a href="whatever.cfm?id=<cfoutput encodeFor="URL">#url.id#</cfoutput>" onclick="get(<cfoutput encodeFor="JavaScript">#url.id#</cfoutput>)">Go to whatever.cfm?id=<cfoutput encodeFor="URL">#url.id#</cfoutput></a>

What a mess. What we could do is this:

<cfoutput>
    #encodeForHtml(url.name)#
    <a href="whatever.cfm?id=#encodeForUrl(url.id)#" onclick="get(#encodeForJavaScript(url.id)#)">Go to whatever.cfm?id=#encodeForUrl(url.id)#</a>
</cfoutput>

Look familiar? Yeah... we already have the correct (and safe) implementation in place.

We don't need to mess with something that's supposed to just output, and make it do something different as well. It's a bad approach to designing the language.

I reckon this ER should be pulled. It's a pity the work has already been done...

Thoughts?

--
Adam

CFML: how should int() behave?

G'day:
One of the scintillating conversations on the CFML Slack channel today was regarding people's expectations of CFML's int() function. Ross had this expression:

int(35 - (1013 / (13+1)))

And he quite legitimately expected the result to be -37 (the float value before converting to an int is -37.357142857143). However because CFML is daft (IMO), its integer conversion follows this rule:

Calculates the closest integer that is smaller than number.

Which means the result of the expression is -38.

What it should do is simply return the integer part of the number. It's not a rounding function, it's a conversion function.

I thought there might be a precedent in other languages, but here's some code samples:

// testInt.java
public class TestInt {

    public static int toInt(Double f){
        return f.intValue();
    }
    
}

Returns 37 when passed -37.357142857143.

Ruby:

irb(main):004:0> -37.357142857143.to_i
=> -37

JavaScript:

parseInt(-37.357142857143)
-37

Groovy:

groovy:000> (int) -37.357142857143
===> -37

Python:

>>> int(-37.357142857143)
-37

Clojure:

user=> (int -37.357142857143)
-37

Oops! Almost forgot PHP (I actually did forget... this is being added during a re-edit)

d:\webSites\www.scribble.local>php
<?php
$f = -37.357142857143;
$i = (int) $f;
echo $i;
^Z
-37

You get the idea.

So... it's a bit late in the game to fix int() in CFML, but John Whish has raised a ticket for ColdFusion: "Deprecate `int` in favour of `floor`" (4044533). This makes good sense to me. CFML already has a ceiling() function, so it makes sense to also have floor().

Note one can use fix() in CFML to get the right result, ie: just the integer part of the number. But that's just a fix. Pity int() didn't just do the right thing in the first place.

Is there any precedent in any other language for an int() operation to actually do rounding instead?

--
Adam

"George", eh?

G'day:
So I heard the first mention of Railo 5 in quite a while from the Railo guys today, an oblique reference from Micha:

In George (Railo 4.2 successor) the release date is set by the build process in the default.properties file, the location for this information has moved because this file is common practice with OSGi.

Two things interest me:

• things have been very quiet re Railo 5George recently, after a lot of initial chatter about it;
• odd to describe it as "Railo 4.2 successor" when previously they've been very open about referring to it as "Railo 5". Dunno what to make of that. I might just be reading too much into casual words on a Google group.

Anyway, it's jolly good to hear mention of it. I just bloody wish they'd hurry up and release the damned thing. At least to a public beta or something! It sounds like it's really going to be a great step forward for CFML, and will probably give Adobe a bit of a fright, I reckon.

Can't wait!

--
Adam


PS: thanks to John Whish for helping me get this article up onto coldfusionbloggers.org, which I cannot access from this machine @ present, for some reason.

Testbox: using a callback as a mocked method

G'day:
John Whish hit me up the other day, wondering had I looked at a new TestBox (well: at the time it was just for MockBox) feature I'd requested and has been implemented. Rather embarrassedly I had forgotten about it, and had not looked at it. That's a bit disrespectful of Luis and/or Brad (or whoever else put the hard work in to implement it), and for that I apologise. However I have looked at it now.

The feature request is this one, MOCKBOX-8:

Luis, we discussed this briefly @ SotR.

Currently one can specify a expression to use for a $results() value, and the value of that expression is then used as the results for the mocked method.

It would be occasionally handy for some stub code to be run to provide the results when a mocked method is called. This could be effected by being able to pass a callback to $results(), which is then called to provide the results when the mocked method itself is called.

Ideally a mocked method should always be able to be fairly dumb, but sometimes in an imperfect world where the mocked method has other dependencies which cannot be mocked out, an actual usable result is necessary. We've needed to do this about... hmmm... 0.5% of the times in out unit tests I think, so this is quite edge-case-y.

Cheers.

I can't recall what exactly we were needing to do at the time, but basically we were testing one method, and that called a different method which we wanted to mock-out. However we still wanted the mocked method to produce "real" results based on its inputs (for some reason).

MockBox allows to mock a method like this:

someObject.$("methodToMock").$results(0,1,2,3,4,etc);

And each call to methodToMock() will return 0, 1, 2 [etc] for each consecutive call, for as many arguments one cares to pass to the $results() method. Once it's used all the passed-in options it cycles back to the beginning again. This is great, but didn't work for us. What we needed was for the methodToMock() to actually run a stub function when it's called. Hence the enhancement request.

The Ortus guys have implemented this, so here's an example of it in action.

Firstly a MXUnit-compat scenario, running on ColdFusion 9 (so no need for function expressions or "closures" as the TestBox docs tends to refer to them as):

// C.cfc
component {

    public struct function getStructWithUniqueKeys(required numeric iterations){
        var struct = {};
        for (var i=1; i <= iterations; i++){
            struct[getId()] = true;
        }
        return struct;

    }

    private string function getId(){
        // some convoluted way of generating a key which we don't want to test
        return ""; // doesn't matter what it is, we won't be using it
    }

}

Here we have a CFC which has a method we want to test, getStructWithUniqueKeys(). It calls getId() to get a unique ID. For whatever reason we don't want our calls to getStructWithUniqueKeys() to call the real getId() (maybe it requires a DB connection or something?), so we want to mock-out getId(). However we still need it to return unique IDs.

Here's our test CFC:

// TestMxUnitCompat.cfc
component extends="mxunit.framework.TestCase" {

    function beforeTests(){
        variables.testInstance = new C();
        new testbox.system.testing.MockBox().prepareMock(testInstance);
        testInstance.$("getId").$callback(mockedGetId);
    }

    function testGetStructWithUniqueKeys(){
        var iterations    = 10;
        var result        = testInstance.getStructWithUniqueKeys(iterations=iterations);
        assertEquals(iterations, structCount(result), "Incorrect number of struct keys created, implying non-unique results were returned from mock");
    }

    private function mockedGetId(){
        return createUuid();
    }

}

Here I'm using Mockbox to prepare my test object for mocking (which just injects a bunch of MockBox helper methods into it), and then we mock getId() so that instead of calling the real getId() method, we use mockedGetId() as a proxy for it. And mockedGetId() just uses createUuid() to return a unique key.

And, pleasingly, this all works fine: