Monday, 18 July 2016

REST & nouns & verbs: analysing the right problem

G'day:
I've recently changed teams at work, and have shifted from "safe" code bases that I've either been working on daily for over a year, or new applications that I've had a hand in from the ground up. Now I'm doing maintenance and adding features to existing code-bases written by different teams for different requirements, developed to different coding guidelines. So that's all a bit of a rude awakening that I am still settling into. I'm definitely out of my comfort-zone, and having to ask different questions than I usually might. That said: it's a good move and I kinda like that sort of thing, so cool!

For better of for worse, we use a lot of REST web services even for our internal processes. I personally believe this is architecturally questionable, but as no-one thought to ask me the question, I didn't get to partake in that particular discussion. To be fair to the situation, it was a decision made before I was part of the department; and - equally - I'm not entirely convinced of my position that it's a daft idea. Just mostly convinced ;-) Anyway, as the over-used truism declares: we are where we are.

Anyway, I find myself doing some maintenance work on one of our web services. This particular service handles the back-end processing for our web applications which make image file uploads. As part of the file upload we need to do a few things:


  • upload the file and stick it in the appropriate place;
  • store some user-provided data about the file (name, description, etc);
  • create a few different resizings of the image for various purposes;
  • contrive some metadata on all the file variations based on [stuff].
  • distribute all the files across our CDN.


The first two of those steps are fast and can be done in real time. The file operations (resizing and distribution) take some time, and obviously the metadata extraction cannot be peformed until the files actually exist.

We have these split into two processes: one that always occurs immediately that uploads the master file and stores it and its user-enter data; then a second process which handles all the slow stuff.  This is just so we can provide fast feedback to the UI. Note that all this is still an atomic operation: both processes need to run.

On the first iteration of the implementation of this the two processes were fired off by the UI. Basically the upload form was submitted and the main request would handle the upload and the data-write, and then a second request was fired off by AJAX. This is not a robust solution as it isn't atomic. It's possible that the first call can be made but the second one doesn't for some reason. From an architectural point of view it's just not up to the view layer to make this call either; the versioning and metadata processing is an adjunct to the main file-handling process on the server: it's more the second half of the same process, not a separate process in its own right. Realistically the two are only separated at all because of performance considerations. From the perspective of the consumer of the web service: it should be one call to the API.

This was driven home to us by a second web app needing to do the same operations with the same web service, and the bods on that team knew about the first call but not the second.

So my task was to remediate this.

The solution we adopted was to have the front-end web app only call the one process, POSTing to http://api.example.com/file, passing along the file-upload form information as the POST body: this is what the UI-driven process was already doing. I simply augmented that to put an async job into our queuing system. The queue job receives the ID of the created file, and then it simply makes a second REST call, also POSTing to another end point: http://api.example.com/process. All good.

A first observation here is that "process" really isn't a great name for a REST end point. An end point should have meaning unto itself, and "process" is meaningless unless one knows it's tightly coupled to an initial end point relating to a /file: "Ah: so it's processing the file". One shouldn't need this sort of context when it comes to a web service end point. Sadly this end point is now out in the wild. Well: here "the wild" is just internal apps, but we can't simply change the URL to be something better without scheduling updates to those apps too. I'll be back-logging work to get that sorted out.

Part of my work here was to boyscout-rule the controllers of the "process" end point to move business logic out of the controller (yeah, don't ask), and into a service tier, which called a repository tier etc.

Some of the business logic I moved was to extract the decision as to which request parameter to use for the ID. Apparently we have some legacy code somewhere which passes the file ID as another argument name - I do not have the code in front of me right now so I can't remember what it is - so there was some logic to determine whether to use the legacy argument or the new one. Arguably this is perhaps still the job of the controller - it's dealing with stuff from the request object, which I can sell to myself as being controlller logic - but we're pretty stringent that we have as little logic in the controller as possible. And as I already needed a service method to do "Other Stuff", so I pushed this back into helper in the service too: something like extractDistributionCriteria(array $allCriteria). I don't want to be passing the entire request object into the service, so I extracted the POST params and passed those, using Silex's $request->request->all() method, and that returns the POST params as an array. Other than that, I was just shuffling logic around, so everything should be grand.

Monday, 16 May 2016

"There, and back again" (or: I suck at JavaScript sometimes)

G'day:
Sometimes I think I'm OK at JavaScript (well: I guess I'm OK at it [he says, whilst seesawing his hand]), and then sometimes I put this to the test and conclude I'm actually a bit shit. In this episode, I cover the journey from former to latter.

Important note:

Given the nature of this article, most of the code in it is wrong, and I am in no-way advocating its usage. The raison d'être of the article is demonstrating bad / wrong / pointless code. Do not copy this code. Do not think the code in here is a solution to any problem you might have.

We've been trying to get coherence in our approach to JavaScript for years. But it always just seems to be a series of false starts, for one reason or another. But I still chip away at trying to improve things where I can. This tends to involve a lot of experimentation, and not much ground made.

A number of years ago we had my mate Chris Kobrzak on the team, and he'd migrated from being a CFML developer to being a dedicated JavaScript developer. This was excellent as he say to it we had a formal JavaScript coding standard, and actively mentored it. During his tenure I think all of us improved our JS no end. However his tenure was short, and he moved onto other challenges after a while. For one reason or another the role was never re-filled (although we needed it more than ever), and our progress on that improvement has stalled. I think we're all still a bunch better than we were then, but this is more down to repetition and attrition more than anything else.

One legacy that Chris left us was a formalised approach to defining JavaScript "classes", wherein we unified on the standard "prototype-based" approach. This won't seem like news, but we had several different variations of inline-objects / kinda-classes / wodges-of-procedural-code going at once. Basically each dev started a new file however they felt like at the time. Or sometimes in the middle of a file, change tactic / style.

The basic rules of the coding standard were:
  • acknowledge that JavaScript is OO but not in the sense we were used to (coming from CFML), and don't bother trying to pretend it's traditional OO by trying to do inheritance or non-supported stuff like that.
  • Namespace everything.
  • We emulated classes using  the prototypical function approach, with a consistency rule that the there was only one class per file, the file was named the same as the class, and the namespace of the class was reflected in the directory hierarchy.
  • Separate the definition of the class from the usage of the class, ie: in different files, indeed different directory trees.
  • Object methods were always added to the main function's prototype.
  • "Class methods" were added to the function directly.
An example of this might be:

var ns = ns || {};

ns.Person = function(firstName, lastName, dob, status){
    this.firstName = firstName;
    this.lastName = lastName;
    this.dob = dob;
    this.status = status;
};

ns.Person.capitaliseName = function(name){
    return name.replace(/(^|\b)([a-z])/g, function(match){return match.toUpperCase();});
}

ns.Person.prototype.getFullName = function(){
    return ns.Person.capitaliseName(this.firstName)
        + " "
        + ns.Person.capitaliseName(this.lastName);
};

ns.Person.prototype.getAgeInYears = function () {
    return new Date().getFullYear() - this.dob.getFullYear();
};

ns.Person.prototype.setStatus = function (status) {
    this.status = status;
};

ns.Person.prototype.getStatus = function () {
    return this.status;
};

ns.Person.prototype.toJSON = function(){
    return {
        firstName : this.firstName,
        lastName : this.lastName,
        fullName : this.fullName,
        dob : this.dob,
        status : this.status
    };
};


Don't worry too much about the code, but here we have a class for a Person which has properties firstName, lastName, date-of-birth, and status. We have object methods to get the person's full name; their age in years; plus set and get a status; and also a method for JSON.stringify() to use when serialising the object. The class also exposes a class method capitaliseName() which can be used for out-of-object situations where one just needs to capitalise a name, but doesn't need an object. The whole thing is namespaced (unimaginatively "ns" in this case) to keep our application's stuff away from other JavaScript stuff.

This file is called Person.js and is at something like /webapp/public/js/lib/ns/Person.js.

It could be used like this:

var person = new ns.Person("kiri", "te kanawa", new Date("1944-03-06"));
console.dir(person);
console.log(person.firstName);
console.log(person.lastName);
console.log(person.getFullName());
console.log(JSON.stringify(person));

console.log(ns.Person.capitaliseName("jean batten"));

var secondPerson = new ns.Person("georgina", "beyer", new Date("1957-11-01"));
console.dir(secondPerson);
console.log(secondPerson.firstName);
console.log(secondPerson.lastName);
console.log(secondPerson.getFullName());

secondPerson.setStatus("Active");
console.log(secondPerson.getStatus());

console.log(JSON.stringify(secondPerson));

Now I hasten to add that all the JavaScript code we're talking about here is client-side JavaScript, I perhaps shoulda mentioned that before. That said, I'm running it via node as the output is easier to copy/paste here:

{ firstName: 'kiri',
  lastName: 'te kanawa',
  dob: Mon Mar 06 1944 00:00:00 GMT+0000 (GMT Standard Time),
  status: undefined }
kiri
te kanawa
Kiri Te Kanawa
{"firstName":"kiri","lastName":"te kanawa","dob":"1944-03-06T00:00:00.000Z"}
Jean Batten
{ firstName: 'georgina',
  lastName: 'beyer',
  dob: Fri Nov 01 1957 00:00:00 GMT+0000 (GMT Standard Time),
  status: undefined }
georgina
beyer
Georgina Beyer
Active
{"firstName":"georgina","lastName":"beyer","dob":"1957-11-01T00:00:00.000Z","status":"Active"}

C:\src>

OK, so that demonstrates everything works A-OK, but also demonstrates something that tends to irk me with JavaScript: for the object methods to access the state of the object, the state needs to be public (accessed via the object's this reference). There's no sense of privacy to the object's properties, and it means all (shared) methods are also really public. In reality this is not often an issue, other than the fact sometimes we find ill-thought-out code battering into a property directly instead of using the class's API. This is a non-theoretical issue: it has caused us problems in the past. Mostly with refactoring. It's always best to keep one's API "on point", as it reduces refactoring challenges.

And it just irks me. I also know I am not alone in this irkery.

One thing some of the bods at work have been looking at recently is YUI's Module Pattern, which kinda deals with the property privacy thing by leveraging closure. Outwardly this sounds all right actually, and when I only fleetingly looked at some example code it looked like the business for us. Here's a reimplementation of the above class using the traditional Module Pattern.

var ns = ns || {};

ns.person = (function(firstName, lastName, dob){
    var fullName = firstName + " " + lastName;
    var private = {
        status : undefined
    };

    var capitalise = function(name){
        return name.replace(/(^|\b)([a-z])/g, function(match){return match.toUpperCase();});
    };

    return {
        getFullName : function(){
            return capitalise(fullName);
        },
        capitaliseName : capitalise,
        getAgeInYears : function () {
            return new Date().getFullYear() - private.dob.getFullYear();
        },
        setStatus : function (status) {
            private.status = status;
        },
        getStatus : function(){
            return private.status;
        },
        toJSON : function() {
            return {
                firstName : firstName,
                lastName : lastName,
                fullName : fullName,
                dob : dob,
                status : private.status
            };
        }
    };
})("jerry", "mateparae", new Date("1954-11-14"));

And some calling code:

console.log(ns.person.getFullName());
console.dir(ns.person);

console.log(ns.person.capitaliseName("temuera morrison"));

ns.person.setStatus("governor general");
console.log(ns.person.getStatus());
console.log(JSON.stringify(ns.person));


And output:
Jerry Mateparae
{ getFullName: [Function],
  capitaliseName: [Function],
  getAgeInYears: [Function],
  setStatus: [Function],
  getStatus: [Function],
  toJSON: [Function] }
Temuera Morrison
governor general
{"firstName":"jerry","lastName":"mateparae","fullName":"jerry mateparae","dob":"1954-11-14T00:00:00.000Z","status":"governor general"}

C:\src>

OK, so this works, in that it hides the properties from the calling code. But - thanks to the IIFE approach - it just creates a one-off object. That's a pretty shit pattern, IMO: the "class" code is not at all reusable... if one wants a second person, one needs to repeat the code. It's also a wee bit shit cos if we want to change the state of any of the private values we need to call them something different from any argument name that might be used to pass-in the values. EG: one cannot do this:

setStatus : function (status) {
    status = status;
},

One needs to do this (or some variation on this, eg have an _status internal variable or something:

ns.Person = function(firstName, lastName, dob, status){
    var private = {
        // ...
        status : status
    };

    // ...

    return {
        // ...
        setStatus : function (status) {
            private.status = status;
        },
        // ...
    };
};

It's not the end of the world, but it's all getting a bit "let's pretend we can do some stuff that we actually can't".

I don't quite know what the thinking was here. We're not gonna be using this approach.

Wednesday, 11 May 2016

JavaScript TDD: getting Mocha tests running on Bamboo

G'day:
We've finally having a chance to move forward with our JavaScript TDD / testing. You might have recalled that I'd lamented in the past we'd wanted to take this sort of thing more seriously with our JS, but it was taking an age to get it moving. Well now it's been placed in the hands of the devs, so it's... moving.

This came to us on Friday, and we had a wee faff around with it then, but didn't make a huge amount of progress. On the weekend I decided to get a proof of concept working, which I did on Saturday. It was kind of a bit of a mission, and I didn't write down what I did so I replicated it again on my other laptop this morning, and took some notes. Which I'll now pad out into an article.

We'd been looking at Jasmine for our JavaScript TDD, but this was based solely on that being the one JS testing framework I was aware of. Jasmine works really well, and superficially I like the approach it has (I say "superficially" cos that's the full extent of my exposure to it). However when I went to see how to get Bamboo running Jasmine tests, all I could really find was chatter about Mocha. I had a quick look at Mocha and it seemed much the same as Jasmine, and looked pretty good, so decided to shift my attention onto using that instead. It seems like Bamboo was kinda pushing me in that direction anyhow.

Installing Mocha is done via NPM:

C:\temp>npm install -g mocha
npm WARN deprecated jade@0.26.3: Jade has been renamed to pug, please install th
e latest version of pug instead of jade
npm WARN deprecated graceful-fs@2.0.3: graceful-fs v3.0.0 and before will fail o
n node releases >= v7.0. Please update to graceful-fs@^4.0.0 as soon as possible
. Use 'npm ls graceful-fs' to find it in the tree.
C:\Users\adam.cameron\AppData\Roaming\npm\mocha -> C:\Users\adam.cameron\AppData
\Roaming\npm\node_modules\mocha\bin\mocha
C:\Users\adam.cameron\AppData\Roaming\npm\_mocha -> C:\Users\adam.cameron\AppDat
a\Roaming\npm\node_modules\mocha\bin\_mocha
mocha@2.4.5 C:\Users\adam.cameron\AppData\Roaming\npm\node_modules\mocha
├── escape-string-regexp@1.0.2
├── commander@2.3.0
├── diff@1.4.0
├── growl@1.8.1
├── supports-color@1.2.0
├── debug@2.2.0 (ms@0.7.1)
├── mkdirp@0.5.1 (minimist@0.0.8)
├── jade@0.26.3 (commander@0.6.1, mkdirp@0.3.0)
└── glob@3.2.3 (inherits@2.0.1, graceful-fs@2.0.3, minimatch@0.2.14)

(don't worry about the deprecation warnings, everything still works... this is all just part of a trademark dispute).

And similarly install Chai and bamboo-mocha-reporter.

Another thing I needed to do is to make sure I had my NODE_PATH environment variable set. I was kinda expecting the Node install to do this, but it didn't. For me that needs to point to C:\Users\adam.cameron\AppData\Roaming\npm\node_modules

So a basic Mocha test looks like this:

var assert = require('chai').assert;

describe('MyClass tests', function() {
    var MyClass = require("../myApp/MyClass");
    describe('Baseline', function () {
        it('should exist and be usable', function () {
            var myObj = new MyClass();
            assert.instanceOf(myObj, MyClass, "myObj should be an instance of MyClass");
        });
    });
});

This is a tweaking of the sample they had on their website's "getting started" section:

var assert = require('chai').assert;
describe('Array', function() {
  describe('#indexOf()', function () {
    it('should return -1 when the value is not present', function () {
      assert.equal(-1, [1,2,3].indexOf(5));
      assert.equal(-1, [1,2,3].indexOf(0));
    });
  });
});

Note that the instructions there don't actually implicitly mention the dependency on Chai here, which is a bit crap of them. They seem to kinda assume one knows this. This is not a safe assumption to make.

Another thing that is "interesting" about Mocha is that it doesn't come with any assertion capabilities at all, so one needs to install Chai (or some other assertions lib) for it to be of much use. I guess this is symptomatic of this mania Node apps have for making everything a "dependency" even if it's really more of a necessity. Equally I discovered that Mocha has no mocking framework either (which almost makes the name of the thing a misrepresentation! ;-), so I think I'll be using Sinon for that. But later.

But anyhow, one one installs all that crap, one can run a test.

I've created a basic GitHub repo, JavaScriptTesting, which has a basic class and a basic test of it:

var Person = function(firstName, lastName) {
    this.firstName = firstName;
    this.lastName = lastName;
};

Person.prototype.getFullName = function(){
    return this.firstName + " " +this.lastName;
};

module.exports = Person;


var assert = require('chai').assert;

describe('Person tests', function() {
    var Person = require("../myApp/Person");
    describe('Baseline', function () {
        it('should exist and be usable', function () {
            var person = new Person();
            assert.instanceOf(person, Person);
        });
    });
    describe('Method tests', function () {
        describe('getFullName tests', function () {
            it('should return a full name', function () {
                var person = new Person("FIRST_NAME", "LAST_NAME");
                var fullName = person.getFullName();
                assert.equal("FIRST_NAME LAST_NAME", fullName);
            });
        });
    });
});

And I can now run these from the command prompt:

C:\src\JavaScriptTesting>mocha


  Person tests
    Baseline
      V should exist and be usable
    Method tests
      getFullName tests
        V should return a full name


  2 passing (47ms)


C:\src\JavaScriptTesting>

Cool. So I know the tests work.

Now I go about getting it all to work on Bamboo. I'd not installed this before so I followed the instructions. One pitfall I fell foul of the first time is the requirements state:

Servlet container requirements
You will need a servlet container that supports the Servlet 2.4 specification. Most modern containers should comply with this.

This is from "Bamboo installation guide". So I messed around getting Tomcat installed (which granted is not much messing around), only to find out that's bullshit. Bamboo installs its own instance of Tomcat, and does that out of the box as part of the installer. Oh well: this is not much of a hardship.

I knew Bamboo will be needing to write stuff to disk and do other stuff that the local system account perhaps did not have permissions to do, so I created a specific account for Bamboo (I called it "bamboo" with a password of "bamboo" ;-), and gave that user full access permissions to the Bamboo home directory.

Oh... during install I let Bamboo create its "home" (/working) directory in the default location which - in hindsight was daft both on my part, and on part of the Bamboo installer, cos it created it in my own user account's user directory. That makes no sense for a server-oriented application. So I changed this to be c:\bamboo.tmp (I'll be blowing all this away on this machine once I've done with this article). So it was that dir I gave the Bamboo user full control permissions to.

After that I dropped down to a command prompt (running as administrator) and ran c:\apps\atlassian\bamboo\InstallAsService.bat to install the Windows service, then I edited that to login with the new bamboo account and started it.

Browsing to http://localhost:8085/ yielded success! (I can't show you a screen shot as I forgot to take one at the time, and I cannot get back to that first screen again now. Just tryst me ;-)

Next Bamboo needs some configuration, so I chose the Express object, which just requires me to enter a login and a password and an email address. Then I was in, and it was time to create a new job to run my tests.

I really have no idea what's going on in Bamboo, but I kinda followed my nose.

I created a "project" called "JavaScript App" which was assigned a short code of "JA". As far as I can tell this is just like in Jira when one creates a new project, and it gets that abbreviation which ends up as the prefix to all tickets. But I dunno. I just agreed with what Bamboo suggested. Next I created a "plan" called "JS TDD CI", and that got an abbrev. of "JTC" (f*** knows if any of this will ever be relevant again, but it's what was going on on the screen as I was thinking "yes, lovely, that's nice").



Then we started to do something that seemed useful: I linked the plan to a repository which is where I point Bamboo to my GitHub repository (that JavaScriptTesting one I mentioned further up). All good.




On the next screen I add the other steps to the plan. It's already added the "Source Code checkout" one for me. So running that much would get the source code checked out.

Next I need to tell Bamboo about Mocha and Chai (and its own mocha-bamboo-reporter), so I add three NPM tasks. Here's the screens for adding the mocha one: the others are all the same:





Having done that, I then add the task to run the tests, which is done via the Mocha Test Runner:



And that's it. I can now run the tests by selecting the run option from the top right of the main plan screen. Stuff whirs into action (and it logs an awful lot of stuff), and eventually:



Now I dart back to my other machine and add a new test to my code:

describe('getAgeInYears tests', function () {
    it('should return the correct age in years', function () {
        var person = new Person("FIRST_NAME", "LAST_NAME", new Date("1970-02-17"));
        var ageInYears = person.getAgeInYears();
        assert.equal(46, ageInYears); // obviously this is a poorly-written test, but it works for a few months yet ;-)
    });
});

This fails cos I have not added the getAgeInYears method yet, but for the sake of demonstration I'll push it up to GitHub.

Bamboo is polling GitHub every three minutes, so after a bit a test run starts and...




There's my latest changes breaking the build. I've never been so pleased to see a failing test before. Hang on whilst I implement that method...



Cool.

Now I did have some problems, and I'm not that happy with a coupla things. Firstly I've got all those NPM modules installed globally already, and I have them on my NODE_PATH. But Bamboo refuses to "see" them. It only seems interested in looking in the node_modules dir in its own working directory. I could not work out how to change that.

I did have some success initially with doing an npm link instead of an npm install in my test script, but when I came to test everything today I must have screwed something up as it had stopped working. Using the install approach requires dragging the stuff down off the 'net each time I do a test run, which seems a bit wasteful. I'm sure it's something I'm doing wrong, and when I work it out, I'll update this article accordingly.

Having got this proof of concept running, we are now in the position to give it a go in our actual work environment (which we've done... we've got our first test passing).

I'm pretty pleased about this. I am now in a place with my JavaScript development that I should have been (and wanted to be) about three years ago. But at least I've got there.

Righto.

--
Adam

Tuesday, 10 May 2016

CFML (yes, shut-up): sorted struct literal syntax

G'day:
Yeah, all right. I'm gonna be admonished by at least one of my mates (that's you, Brian) for writing another article about CFML, but... oh well. This is not gonna become a habit as I feel bad writing it.

You might remember that CF2016 promised to have sorted structs. I documented this ages ago "ColdFusion 2016: ordered/sorted structs". One thing you might have noticed is that whilst ordered structs made the cut; sorted ones did not. Adobe's briefly documented the ordered ones here: "Collection support (Ordered)".

To recap: structs in CFML have always been intrinsically unordered collections. If you want an ordered collection: use an array. Structs to preserve an internal ordering, but how that's done is specifically undocumented and one should never rely on it. Previously if one's needed to access a struct's values in a specific order, one's needed to maintain a separate index of what that order is, or just use the sort method to return an array of they keys in a prescribed order and then iterate over that, using each array element value to access the struct. It's all in the docs. The chief problem with sorting a struct was it was only ever based on the keys, and only ever in a lexical ordering. Which is almost never a sensible ordering to use. But this is just the penalty one pays for trying to use an intrinsically unordered data type as an ordered one.

ColdFusion 2016 added ordered structs, and was supposed to add sorted ones too, but sorted ones did not make the cut for release (and that's as much as I can say about that due to NDA reasons). I had pointed out to Adobe that "ordered" and "sorted" means the same thing for all intents and purposes... I don't actually know if they've taken this on board. I hope so, so they don't just look like dicks when they do get around to releasing the sorted variety.

I do feel safe to say they do intend to finish this particular job... I don't think that's breaking any NDA. I cannot say when any updates to ColdFusion 2016 might come out though, as I'm completely out of that loop now. Except for elements of the matter that I will not refer to directly, but you can read between the lines that they're perhaps under discussion somewhere.

Now... the contrived difference in definition that Adobe have invented between "ordered" and "sorted" is that an ordered struct preserves the order in which the keys are added:

person = [firstName="Kiri", lastName="Te Kanawa"];

If one iterates over that, we get Dame Kiri's name-order preserved:

C:\Users\adam.cameron>cf
person = [firstName="Kiri", lastName="Te Kanawa"];
person.each(function(key, value){
CLI.writeln("#key#: #value#");
});
^Z

FIRSTNAME: Kiri
LASTNAME: Te Kanawa
C:\Users\adam.cameron>

Compare that to a normal struct:


C:\Users\adam.cameron>cf
person = {firstName="Kiri", lastName="Te Kanawa"};
person.each(function(key, value){
    CLI.writeln("#key#: #value#");
});
^Z

LASTNAME: Te Kanawa
FIRSTNAME: Kiri
C:\Users\adam.cameron>

You see the key order is not preserved.

OK, this is all lovely, but this is about sorted structs. So what's the diff? To recap, a Sorted struct is one in which one can specify the order the keys should iterate in. This is a bit edge-case-y, but one might want to iterate over they values in some different order other than insertion or key-name order. Perhaps alphabetically by value. Or by some other enumeration, or by length of value or something. There's a use case there, even if it's also edge-case.

One of the big challenges with sorted structs is the literal expression syntax for them. You know: array literals are expressed like this:

["tahi", "rua", "toru", "wha"]

Structs:

{red="whero", green="kakariki", blue="kikorangi"}

And now ordered structs:

[firstName="Kiri", lastName="Te Kanawa"]

Sorted structs need to impart two different things:
  • the key/value pairs
  • the sorting mechanism

Within the sorting mechanism, there are some variations:
  • declaratively: sorting ascending, alphabetically, using a specific locale as the collation order
  • functionally: using a callback 

So how the hell to impart all that info in a literal?

There's been a bunch of suggestions (none of which I can share, sorry), but none have been complete or "natural-seeming". Abram floated some suggestions in a bug ticket - 4126544 - which are good suggestions, but not really solving the "literal" problem.

But I was discussing this tonight with my mate and ColdFusion testing machine Aaron Neff and we finally nailed a decent syntax I think (I await Sean to tell me otherwise ;-)

For the declarative approach:

sortedStruct = [key=value, key=value, struct]

Where the struct argument has the config details for the sort:

{direction="ASC|DESC", on="key|value", type="text|numeric", caseSensitive=boolean(false), locale="fr_FR"}

Put together as an example:

sortedStruct = [january=29, february=17, march=31, {type="numeric", on="value"}];

This would yield a struct that when iterated over would return the key/values in order Feb, Jan, March, as it's sorting on value (ascending is implicit given it'd be the default for that setting).

And for using a callback, instead of a struct one just gives the function:

sortedStruct = [key=value, key=value, function(element1, element2, struct)]

 eg:

sortedStruct = [key=value, key=value, function(element1, element2, struct)

{
    var monthLengths = [january=31, february=28, march=31];
    return element1.value / monthLengths[element1.key] - element2.value / monthLengths[element2.key];
}];

This sorts the struct on the proportionate daily value per month. Oh of course the comparator function returns one of <1, 0 or >1 as per all sorting comparator functions. I don't use the third argument to the callback in this example, but that's there to mirror other iteration callbacks which take a reference the whole collection as the last argument (should one need it).

I think that's a reasonable literal syntax for sorted structs, don't you? The only slight quirk is that there's the "special treatment" of the "last" item of the list of elements in the brackets, but I don't mind that. I suppose we could do this:

sortedStruct = [key=value, key=value](struct|callback)

?


But I think either syntax would be unambiguously parseable by ColdFusion, and I think it's clear what's going on when one eyeballs the code.

What do you think? Shall I float this as an idea?

Now... back to doing something that is a good use of my time [cracks another beer, starts Fallout 4].

Righto.

--
Adam

Wednesday, 4 May 2016

PHP: constants vs private static variables and understanding the intent of code

G'day:
I was looking at some code the other day, and got into a conversation with the author about it. I can't show you the actual code (and, indeed, I'll be fictionalising the content of the conversation for editorial reasons), but here's a contrived reproduction of the situation.

There is a function:

function someFunction($arg1, $arg2) {
    $intermediaryValue = $this->firstDependency->prepare($arg1);
    return $this->secondDependency->process($intermediaryValue, $arg2);
}

Don't worry about that, what I was looking at were the tests for the function. Those dependencies needed to be mocked-out, and for the purposes of this function, it's important that what's returned from someFunction is exactly the result of the process method. So we need to specifically test for that.

class SomeClassTest extends \PHPUnit_Framework_TestCase {

    const MOCKED_PREPARE_RESULT = 'MOCKED_PREPARE_RESULT';
    const MOCKED_PROCESS_RESULT = 'MOCKED_PROCESS_RESULT';

    private $mockedFirstDependency;
    private $mockedSecondDependency;
    private $someObject;
    
    
    function setup(){
        $this->mockedFirstDependency = $this->getMockedFirstDepenedency();
        $this->mockedSecondDependency = $this->getMockedSecondDepenedency();
        $this->someObject = new SomeClass($mockedDependency);
    }
    
    /**
    * @dataProvider provideCasesForSomeFunctionTests
    */
    function testSomeFunction($testArg1, $testArg2){
        $this->mockedFirstDependency->method('prepare');
            ->with($testArg1, $testArg2)
            ->willReturn(self::MOCKED_PREPARE_RESULT);
            
        $result = $this->someObject->someFunction($testArg1, $testArg2);
        
        $this->assertEquals(self::MOCKED_PROCESS_RESULT, $result);
        
    }
    
    function provideCasesForSomeFunctionTests(){
        return [
            'first variation' => ['testArg1'=>'test value 1', 'testArg2'=>'test value 2'],
            'second variation' => ['testArg1'=>'test value 3', 'testArg2'=>'test value 4']
        ];
    }
    
    private function getMockedFirstDependency(){
        $mockedFirstDependency = $this->getMockBuilder('\some\namespace\FirstDependency')
            ->setMethods(['prepare'])
            ->getMock();
            
        return $mockedFirstDependency;
    }
    
    private function getMockedSecondDependency(){
        $mockedSecondDependency = $this->getMockBuilder('\some\namespace\SecondDependency')
            ->setMethods(['process'])
            ->getMock();
        
        $mockedSecondDependency->method('process')
            ->with(self::MOCKED_PREPARE_RESULT)
            ->willReturn(self::MOCKED_PROCESS_RESULT);
            
        return $mockedSecondDependency;
    }

}

What I was specifically looking at was the usage of constants here. Chiefly I was wondering "why are those constants?" I scanned the rest of the code and they were only ever used in this class (and remember it's a test class), and the names and the values are very specific to their usage in said class.

So I asked. The reason why they're constants is because they're unchanging values. Hmmm. Well... that doesn't wash, because a variable that one simply doesn't change also doesn't change value. So that's not valid. There is more to that necessary to determine something ought to be a constant.

Firstly, in PHP - until PHP7 anyhow - constants are intrinsically public. And according to the principle of encapsulation and the notion of information hiding, the only thing in a class that ought to be public is stuff that is specifically supposed to be public. IE: the API to the class. By inference, if you make something public, it's a flag saying "use this publicly... it's there specifically for you to do that". That is not valid here, so it's invalid for the value to be public, so - even if the value/usage was appropriate to be a constant - it kinda can't be one.

Secondly... the usage here simply isn't that of a constant. The rationalisation that its value is a constant/static/literal value therefore it should be a constant is specious. That's not how it works. Let's have a look at how we ended up with this constant being created.

Firstly we had this pre-refactoring version of the code:

function testSomeFunction($testArg1, $testArg2){
    $this->mockedFirstDependency->method('prepare');
        ->with($testArg1, $testArg2)
        ->willReturn('MOCKED_PREPARE_RESULT');
        
    $result = $this->someObject->someFunction($testArg1, $testArg2);
    
    $this->assertEquals('MOCKED_PROCESS_RESULT', $result);
    
}

// [...]

private function getMockedSecondDependency(){
    $mockedSecondDependency = $this->getMockBuilder('\some\namespace\SecondDependency')
        ->setMethods(['process'])
        ->getMock();
    
    $mockedSecondDependency->method('process')
        ->with('MOCKED_PREPARE_RESULT')
        ->willReturn('MOCKED_PROCESS_RESULT');
        
    return $mockedSecondDependency;
}


Note how we have a coupled duplicated values there: MOCKED_PREPARE_RESULT and MOCKED_PROCESS_RESULT. That's not DRY, so obviously there's some refactoring to do. Before we refactor, let's pause for a second. Imagine the code was like this:

function testSomeFunction($testArg1, $testArg2){
    $this->mockedFirstDependency->method('prepare');
        ->with($testArg1, $testArg2)
        ->willReturn('GENERIC_DUMMY_VALUE');
        
    $result = $this->someObject->someFunction($testArg1, $testArg2);
    
    $this->assertEquals('GENERIC_DUMMY_VALUE', $result);
    
}

We have the same duplication twice (he says, being deliberately tautological) in the same function. What do we do? We extract it to a variable:

function testSomeFunction($testArg1, $testArg2){
    $genericDummyValue = 'GENERIC_DUMMY_VALUE';
    $this->mockedFirstDependency->method('prepare');
        ->with($testArg1, $testArg2)
        ->willReturn($genericDummyValue);
        
    $result = $this->someObject->someFunction($testArg1, $testArg2);
    
    $this->assertEquals($genericDummyValue, $result);
    
}

Now all things being equal, that's a fine enough solution. If it was in a "real" class as opposed to just a test one might conclude that having a magic value in the middle of the code like that might be poor play, but in this case it's fine.

But in our case, the usage of the values is spread across two functions, so using function-local variables doesn't help it. What do we do when we need the same variable across more than one function? We hoist it up to be a class variable (well, two: $mockedPrepareResult and $mockedProcessResult):

private $mockedPrepareResult = 'MOCKED_PREPARE_RESULT';
private $mockedProcessResult = 'MOCKED_PROCESS_RESULT';

// [...]

function testSomeFunction($testArg1, $testArg2){
    $this->mockedFirstDependency->method('prepare');
        ->with($testArg1, $testArg2)
        ->willReturn($this->mockedPrepareResult);
        
    $result = $this->someObject->someFunction($testArg1, $testArg2);
    
    $this->assertEquals($this->mockedProcessResult, $result);
    
}

// [...]

private function getMockedSecondDependency(){
    $mockedSecondDependency = $this->getMockBuilder('\some\namespace\SecondDependency')
        ->setMethods(['process'])
        ->getMock();
    
    $mockedSecondDependency->method('process')
        ->with($this->mockedPrepareResult)
        ->willReturn($this->mockedProcessResult);
        
    return $mockedSecondDependency;
}

And one might go one step further and conclude these variables are indeed class variables and not instance variables, so one makes them static as well:

private static $mockedPrepareResult = 'MOCKED_PREPARE_RESULT';

// [...]

    $this->mockedFirstDependency->method('prepare');
        ->with($testArg1, $testArg2)
        ->willReturn(self::$mockedPrepareResult);

Bear in mind that in common English "constant" and "static" are analogous, but in programming usage they are for the most part unrelated. A constant is an immutable value. a static value is one that is specific to the class it's in, rather than an instance of that class. There's no overlap in the two notions in either of those contexts the terms have meaning.

So as to why one would not then go one step further and change them from private static variables to constants... it's because that's a complete non-sequitur. At no point is there any indication that the usage of these values here implies that they're supposed to be constants. They're just variables, and in the usage of these variable it would be a logic error to change the value between separate references to the variable. This is analogous to this code:


$theTotal = $this->getTheTotal();
$theTotal += 1;
return $theTotal;

It'd be a logic error to increment $theTotal by one, so don't bloody do it. It's not down to a language construct to prevent this, it's down to the programmer not being a div.

I doubt any developer would do this:

final $theTotal = $this->getTheTotal();
$theTotal += 1;
return $theTotal;

(pre-supposing PHP has the final keyword, which it doesn't). Thus putting it down to the compiler to catch the logic error.

What's a constant then?

Well whether or something ought to be a constant is down to two things. First the constant's value. It's not so much that "one oughtn't change it" (although preventing that is a side effect of something being a constant), it's that the value of the constant doesn't change.

This is a sensible usage of constants:

class Numbers {

    const BRACE = 2;
    const DOZEN = 12;
    const SCORE = 20;
    const GROSS = 144;
    
    // [...]
}

It's not so much that one doesn't want to change the value of brace, it's that the value of what it is to be a brace doesn't change. It's constant. That's when one would use a constant.

The second criterion as to why one might make a value a constant is that one actively wants to expose it publicly. A good real-world example of constants are in Symfony 2's Response class:

class Response
{
    const HTTP_CONTINUE = 100;
    const HTTP_SWITCHING_PROTOCOLS = 101;
    const HTTP_PROCESSING = 102;            // RFC2518
    const HTTP_OK = 200;
    const HTTP_CREATED = 201;
    const HTTP_ACCEPTED = 202;
    const HTTP_NON_AUTHORITATIVE_INFORMATION = 203;
    const HTTP_NO_CONTENT = 204;
    const HTTP_RESET_CONTENT = 205;
    // etc you get the idea

So when creating responses in our controllers, we don't do stuff like:

return Response::create($twig->render('clientError.twig'), 404);

Instead we do this:

return Response::create($twig->render('clientError.twig'), Response::HTTP_NOT_FOUND);

It just make the code more clear as to what's going on.

Also note that these example usages are all publicly exposed, and that's their intended usage. In that Numbers class if we only ever used the notion of a BRACE internally, and didn't have reason to expose it (it's a not oft-used word, after all), they we'd not make a constant, we'd just have a private static variable.

Realistically constants are something that one generally won't want to use. It's only in those rare situations where one has a literal value which has a specific constant meaning, and could benefit from being given a human-friendly (/code-clarity-friendly) label. Most values one will want to expose from a class or an object will not be simple literal values, so one would be exposing them via methods.

And it's a misunderstanding of the intended purpose of constant to conclude they're for literal values you're unlikely to want to change (in just a logical sense). They're for literal values which don't. That's a different thing.

Righto.

--
Adam

Sunday, 1 May 2016

PHP: Scaling the decorator pattern

G'day:
I while back I had a look at using the Decorator Pattern to simplify some code:


One again me mate Brian has come to the fore with a tweak to make this tactic an even more appealing prospect: leveraging PHP's "magic" __call method to simplify decorator code.

Let's have a look at one of the examples I used earlier:

class User {

    public $id;
    public $firstName;
    public $lastName;

    function __construct($id, $firstName, $lastName){
        $this->id = $id;
        $this->firstName = $firstName;
        $this->lastName = $lastName;
    }

}

class DataSource {

    function getById($id){
        return json_encode([
            'id' => $id,
            'firstName' => 'Zachary',
            'lastName' => 'Cameron Lynch',
        ]);
    }
}

class LoggerService {

    function logText($text){
        echo "LOGGED: $text" . PHP_EOL;
    }
}


class UserRepository {

    private $dataSource;

    public function __construct($dataSource) {
        $this->dataSource = $dataSource;
    }

    public function getById($id) {
        $userAsJson = $this->dataSource->getById($id);

        $rawUser = json_decode($userAsJson);
        $user = new User($rawUser->id, $rawUser->firstName, $rawUser->lastName);

        return $user;
    }

}

class LoggedUserRepository {

    private $repository;
    private $loggerService;

    public function __construct($repository, $loggerService) {
        $this->repository = $repository;
        $this->loggerService = $loggerService;
    }

    public function getById($id) {
        $this->loggerService->logText("$id requested");
        $object = $this->repository->getById($id);

        return $object;
    }

}

$dataSource = new DataSource();
$userRepository = new UserRepository($dataSource);

$loggerService = new LoggerService();
$loggedUserRepository = new LoggedUserRepository($userRepository, $loggerService);

$user = $loggedUserRepository->getById(5);
var_dump($user);

That looks like a chunk of code, but the User, DataSource and LoggerService are just dependencies. The code you really wannna look at are the two repo variations: the basic UserRepository, and the decorated LoggedUserRepository.

This code all works fine, and outputs:

C:\src>php baseline.php
LOGGED: 5 requested
object(User)#6 (3) {
  ["id"]=>
  int(5)
  ["firstName"]=>
  string(7) "Zachary"
  ["lastName"]=>
  string(13) "Cameron Lynch"
}

C:\src>

Fine.

But this is a very simple example: the repo has only one one method. So the decorator only needs to implement one method. But what if the repo has ten public methods? Suddenly the decorator is getting rather busy, as it needs to implement wrappers for those methods too. Yikes. This is made even worse if the decorator is only really interested in decorating one method... it still needs those other nine wrappers. And it'd be getting very boiler-plate-ish to have to implement all these "empty" wrapper methods.

Let's add a coupla more methods to our repo:

class UserRepository {

    private $dataSource;

    public function __construct($dataSource) {
        $this->dataSource = $dataSource;
    }

    public function getById($id) {
        $userAsJson = $this->dataSource->getById($id);

        $rawUser = json_decode($userAsJson);
        $user = new User($rawUser->id, $rawUser->firstName, $rawUser->lastName);

        return $user;
    }

    public function getByFilters($filters) {
        $usersAsJson = $this->dataSource->getByFilters($filters);

        $rawUsers = json_decode($usersAsJson);
        $users = array_map(function($rawUser){
            return new User($rawUser->id, $rawUser->firstName, $rawUser->lastName);
        }, $rawUsers);

        return $users;
    }

    public function create($firstName, $lastName){
        $rawNewUser = json_decode($this->dataSource->create($firstName, $lastName));
        return new User($rawNewUser->id, $rawNewUser->firstName, $rawNewUser->lastName);
    }
}

We have two new methods: getByFilters() and create(). Even if we don't want to log calls to those for some reason, we still need the LoggedUserRepository to implement "pass-through" methods for them:

class LoggedUserRepository {

    private $repository;
    private $loggerService;

    public function __construct($repository, $loggerService) {
        $this->repository = $repository;
        $this->loggerService = $loggerService;
    }

    public function getById($id) {
        $this->loggerService->logText("$id requested");
        $object = $this->repository->getById($id);

        return $object;
    }

    public function getByFilters($filters) {
        return $this->repository->getByFilters($filters);
    }

    public function create($firstName, $lastName) {
        return $this->repository->create($firstName, $lastName);
    }

}


See how we still need methods for getByFilters and create? Suck. I mean it's not a huge amount of code, but given the LoggedUserRepository doesn't actually wanna log those methods, they're out of place.

Saturday, 30 April 2016

PHP: Strategy pattern to defer implementation details

G'day:
Once again my Saturday evening is set in the local pub (well: the one in Galway I go to when I'm here anyhow. I go to this place more than I do my "local" at home), with a Guinness and the laptop in front of me. I've not actually done this for a while, coming to think of it.

Right, so today's random code exercise is brought to me via Stack Overflow, with some bod asking a question about how to decouple knowledge of the implementation of a task from the code that needs to execute the task. They ask it in a slightly different way, but that's the gist of it. Go read it anyhow.

This very thing has come up at work a bit recently... we have a tendency to have a single class which has various different implementations of the same task, but for different situational variations. We could all see this was a bit of a smell, but weren't initially sure how we should approach it. Enter my mate Brian and his ability to remember things that he's read (damn him), and at some stage he's read that - apparently turgid to the point of being close to unreadable - book "Design Patterns [etc]" which at least identifies a bunch of design patterns we should all be aware of when solving problems. I think I'll stick with the copy of "Head First Design Patterns" sitting on my desk. It's eminently readable.

Right, so this sounds like a case for the Strategy Pattern, which - according to Wikipedia - "… is a software design pattern that enables an algorithm's behavior to be selected at runtime". Basically you have a class which has a method to perform a task, but the implementation of the task is handed off to a collaborator which is implemented separately.

By way of example I'm gonna use a very contrived situation to demonstrate the technique. NB: this is different from my answer on Stack Overflow, but amounts to the same thing. Here we the notion of a Name, and a NameDecorator; and the requirement is to render the name in two ways: plain text as "[lastName], [firstName]" and with a mark-up template to effect something like "[firstName] [lastName]". A naïve implementation might be as follows:

First we just have a very generic Name class:

class Name {

    public $firstName;
    public $lastName;

    function __construct($firstName, $lastName){
        $this->firstName = $firstName;
        $this->lastName = $lastName;
    }
}

That's not really part of the problem here, but the code revolves around this. And now our Decorator class:

class NameDecorator {

    private $name;
    private $fullNameTemplate;

    function __construct($name, $fullNameTemplate){
        $this->name = $name;
        $this->fullNameTemplate = $fullNameTemplate;
    }

    function renderFullNameAsPlainText(){
        return sprintf('%s, %s', $this->name->lastName, $this->name->firstName);
    }

    function renderFullNameWithHtmlTemplate(){
        return sprintf($this->fullNameTemplate, $this->name->firstName, $this->name->lastName);
    }
}

And sample usage:

$name = new Name('Zachary', 'Cameron Lynch');
$template = '%s <strong>%s</strong>';

$nameDecorator = new NameDecorator($name, $template);

$rendered = $nameDecorator->renderFullNameAsPlainText();
echo "Simple rendering: $rendered<br>";

$rendered = $nameDecorator->renderFullNameWithHtmlTemplate();
echo "Rendering with template: $rendered<br>";

And result:
Simple rendering: Cameron Lynch, Zachary
Rendering with template: Zachary Cameron Lynch


Here's the problem. Sometimes we need to render the name using a template: sometimes we don't. Some entire usages of this decorator will never need the template, but we still need to initialise the decorator with it.

This could be easily solved by passing the template with the rendering call, instead of init-ing the whole object with it:

function renderFullNameWithHtmlTemplate($template){
    return sprintf($template, $this->name->firstName, $this->name->lastName);
}

And if that was the full requirement, that'd be fine. But the requirement is such that we need to call this code from other code which doesn't want to fuss about with templates and which method to call, it just wants to call a render method and dependent on how the decorator has been configured, will use whatever rendering implementation the decorator has decided upon.

We could solve this with an abstract notion of what the NameDecorator is, and have concrete implementations for each variation:

abstract class NameDecorator {

    protected $name;

    function __construct($name){
        $this->name = $name;
    }

    abstract public function render();
}

class PlainTextNameDecorator extends NameDecorator {

    function render(){
        return sprintf('%s, %s', $this->name->lastName, $this->name->firstName);
    }
}

class TemplatedNameDecorator extends NameDecorator {

    protected $fullNameTemplate;

    function __construct($name, $fullNameTemplate){
        parent::__construct($name);
        $this->fullNameTemplate = $fullNameTemplate;
    }

    function render(){
        return sprintf($this->fullNameTemplate, $this->name->firstName, $this->name->lastName);
    }
}

Now we just use separate Decorator classes: PlainTextNameDecorator and TemplatedNameDecorator, but we have the one unified render() method:

$name = new Name('Zachary', 'Cameron Lynch');

$nameDecorator = new PlainTextNameDecorator($name);
$rendered = $nameDecorator->render();
echo "Simple rendering: $rendered<br>";

$template = '%s <strong>%s</strong>';
$nameDecorator = new TemplatedNameDecorator($name, $template);
$rendered = $nameDecorator->render();
echo "Rendering with template: $rendered<br>";

TBH, if the full requirement was as I've stated here, that'd probably do. But... same as with my discussion "Decorator Pattern vs simple inheritance", it's not very scalable. This works for one variation, but the number of inherited classes gets out of hand very quickly with each method that potentially needs its implementation abstracted. For example if one had a Person class and two methods which need handling: renderName() and renderAddress()... to cover the bases with "plain text" and "templated" options, one would need classes like this:
  • PlainNamePlainAddressPersonDecorator
  • PlainNameTemplatedAddressPersonDecorator
  • TemplatedNamePlainAddressPersonDecorator
  • TemplatedNameTemplatedAddressPersonDecorator

And if we wanted to include a phone number? Eight classes. Basically it's m^n classes, where m is the ways to decorate things, and n is the number of items needing decoration. So if we also had a need to provide for JSON decoration for these three: up from eight to 27 possible variations. Yikes. Not. Scalable.

A more scalable approach is to use composition over inheritance (the more I program, the more I find this to be the case), and use the Strategy Pattern. This basically means you extract just the method functionality into a collaborator, and just tell your main class about the collaborator.

class NameDecorator {

    private $renderingHandler;
    private $name;

    function __construct($name, $renderingHandler){
        $this->name = $name;
        $this->renderingHandler = $renderingHandler;
    }

    function render(){
        return $this->renderingHandler->render($this->name->firstName, $this->name->lastName);
    }
}

Here NameDecorator knows it can render stuff, but it itself has no idea or care about the detail of how to render stuff. All it is doing is exposing a uniform API to the calling code: it can simply call render():

$rendered = $nameDecorator->render();
echo "Simple rendering: $rendered<br>";

Before doing that though, we need to initialise the decorator with a handler:

$nameDecorator = new NameDecorator($name, new DefaultRenderingHandler());

And it's the handler that knows its own various pre-requisites:

class DefaultRenderingHandler {

    function render($firstName, $lastName){
        return "$lastName, $firstName";
    }
}

class TemplatedRenderingHandler {

    private $template;

    function __construct($template){
        $this->template = $template;
    }

    function render($firstName, $lastName){
        return sprintf($this->template, $firstName, $lastName);
    }
}

These classes just focus on what it is to render a name. The default one just gets on with it, whereas the templated one needs to be initialised with a template. But these sort of conceits are limited to the element that needs them: the Decorator itself doesn't need to know the templated handler needs a template. All it needs is a collaborator which implements a render(firstName, lastName) method.

If we have further rendering requirements (say an address, as per earlier), we just need the collaboration for that. So the scaling for this model is m*n, not m^n. Obviously better. Of course one might not need every variation, but it's more the rapidity of things getting out of hand which is the important point here.

It also keeps things more focused: each collaborator simply needs to focus on what it needs to deal with, and not anything else. And the decorator itself just worries about providing its API.

It might seem like a lot of work to create a whole class for a single method, but - really - the class boilerplating isn't much is it? One could use stand-alone functions for this:

class NameDecorator {

    private $renderingHandler;
    private $name;

    function __construct($name, $renderingHandler){
        $this->name = $name;
        $this->renderingHandler = $renderingHandler;
    }

    function render(){
        return ($this->renderingHandler)($this->name->firstName, $this->name->lastName);
    }
}


$renderAsPlainText = function ($firstName, $lastName) {
    return "$lastName, $firstName";
};

$template = '%s <strong>%s</strong>';

$renderWithTemplate = function ($firstName, $lastName) use ($template) {
    return sprintf($template, $firstName, $lastName);
};


$name = new Name('Zachary', 'Cameron Lynch');

$nameDecorator = new NameDecorator($name, $renderAsPlainText);
$rendered = $nameDecorator->render();
echo "Simple rendering: $rendered<br>";


$nameDecorator = new NameDecorator($name, $renderWithTemplate);
$rendered = $nameDecorator->render();
echo "Rendering with template: $rendered<br>";

See how I'm just using functions instead of whole classes here? But that's pretty poor code organisation IMO, so just stick 'em in classes.

The original question on Stack Overflow has asked for a variation and I think the Factory Method Pattern might be the way to go there. But... I'll think about that over night and perhaps come up with something tomorrow. I've got a bunch of downtime (this is for you, Andy Myers) at the airport waiting for a flight tomorrow afternoon.

Righto.

--
Adam

PS: four pints, that one.