Friday 30 December 2016

PHP: using SoapClient to consume a SOAP web service

G'day:
Yeah, OK, just don't ask why I'm needing to consume SOAP web services. It's Adobe's fault. Not because of ColdFusion for a change, but because an app they offer which only exposes its API via SOAP. Because it's still 2006 at Adobe, it seems.

For the last few weeks, this has been running through my head:



But anyway.

Fighting with the Adobe web service was a right pain in the arse, but I won't go into that - it's not useful info to share - but I did have to monkey around with how PHP consumes SOAP web services, and piecing it together took a chunk of googling for me to get my brain around it, so I thought I'd write it up here.

I hasten to add that most of the hassle getting it to work was because of the shitty Adobe web service being unhelpful, so it's perhaps not a challenge for most other people.

I cannot deal with the Adobe web service in this article as it's subscription-only, and it's kinda work-related stuff I'm not allowed to expose here. So I knocked together my own web service to consume. I used CFML for this as all it takes to create a SOAP web service in CFML is to give a method an access qualifier of remote, and have it web-accessible. ColdFusion handles everything else.

Here's the web service code (I'll keep the CFML code to a minimum, but it's all on GitHub anyhow):

import me.adamcameron.accounts.*;

component wsversion=1 {

    remote Invoice function getById(numeric id) returnformat="wddx" {
        var address = new Address(1, "London", "United Kingdom", "E18");

        var account = new PersonalAccount(2, "Adam", "Cameron", "1970-02-17", address);

        var penguin = new Product(3, "Penguin", 4.56);
        var pangolin = new Product(7, "Pangolin", 8.90);
        var platypus = new Product(11, "Playtpus", 12.13);

        var lines = [
            new InvoiceLine(14, penguin, 15, 16.17),
            new InvoiceLine(18, pangolin, 19, 20.21),
            new InvoiceLine(22, platypus, 23, 24.25)
        ];

        invoice = new Invoice(id, account, lines);

        return invoice;        
    }

}

Note that I should not have to state the wsversion there, but for some reason I could not get the default - version 2 - to work, so I had to force it to be version 1. I didn't look into why it didn't work on version 2, as I really didn't want to spend more time than necessary on the CFML code for this exercise.

Other than that, this code is straight forward: it returns an Invoice object which comprises a Person (with an Address), and an array of InvoiceLines each of which have a Product. the modelling for those is the same as in the receiving PHP code, further down.

On the PHP side of things, I just call this code to consume it:

<?php

namespace me\adamcameron\accounts;

require __DIR__ . '/model.php';

$wsdl = "http://localhost:8516/cfml/webservices/soap/accounts/public/Invoices.cfc?wsdl";


$options = [
    'trace' => true
];

$client = new \SoapClient($wsdl, $options);

$invoice = $client->getById(2011);

var_dump($invoice);
echo "==============================" . PHP_EOL . PHP_EOL;
var_dump($client->__getLastResponse());

So it's simply a matter of creating a SoapClient with the WSDL URL and some options. I'm using the trace option here so I can do that call to __getLastResponse.

Let's have a look at the raw SOAP response first (warning: it's pretty turgid reading):

<?xml version="1.0" encoding="UTF-8"?>
<soapenv:Envelope xmlns:soapenv="http://schemas.xmlsoap.org/soap/envelope/" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
    <soapenv:Body>
        <ns1:getByIdResponse xmlns:ns1="http://public.accounts.soap.webservices.cfml" soapenv:encodingStyle="http://schemas.xmlsoap.org/soap/encoding/">
            <getByIdReturn xmlns:ns2="http://accounts.adamcameron.me" xsi:type="ns2:Invoice">
                <account xsi:type="ns2:PersonalAccount">
                    <address xsi:type="ns2:Address">
                        <country xsi:type="xsd:string">United Kingdom</country>
                        <id xsi:type="xsd:double">1.0</id>
                        <localPart xsi:type="xsd:string">London</localPart>
                        <postcode xsi:type="xsd:string">E18</postcode>
                    </address>
                    <dateOfBirth xsi:type="xsd:string">1970-02-17</dateOfBirth>
                    <firstName xsi:type="xsd:string">Adam</firstName>
                    <id xsi:type="xsd:double">2.0</id>
                    <lastName xsi:type="xsd:string">Cameron</lastName>
                </account>
                <id xsi:type="xsd:double">2011.0</id>
                <items xmlns:soapenc="http://schemas.xmlsoap.org/soap/encoding/" soapenc:arrayType="xsd:anyType[3]" xsi:type="soapenc:Array">
                    <items xsi:type="ns2:InvoiceLine">
                        <count xsi:type="xsd:double">15.0</count>
                        <id xsi:type="xsd:double">14.0</id>
                        <price xsi:type="xsd:double">16.17</price>
                        <product xsi:type="ns2:Product">
                            <description xsi:type="xsd:string">Penguin</description>
                            <id xsi:type="xsd:double">3.0</id>
                            <rrp xsi:type="xsd:double">4.56</rrp>
                        </product>
                    </items>
                    <items xsi:type="ns2:InvoiceLine">
                        <count xsi:type="xsd:double">19.0</count>
                        <id xsi:type="xsd:double">18.0</id>
                        <price xsi:type="xsd:double">20.21</price>
                        <product xsi:type="ns2:Product">
                            <description xsi:type="xsd:string">Pangolin</description>
                            <id xsi:type="xsd:double">7.0</id>
                            <rrp xsi:type="xsd:double">8.9</rrp>
                        </product>
                    </items>
                    <items xsi:type="ns2:InvoiceLine">
                        <count xsi:type="xsd:double">23.0</count>
                        <id xsi:type="xsd:double">22.0</id>
                        <price xsi:type="xsd:double">24.25</price>
                        <product xsi:type="ns2:Product">
                            <description xsi:type="xsd:string">Playtpus</description>
                            <id xsi:type="xsd:double">11.0</id>
                            <rrp xsi:type="xsd:double">12.13</rrp>
                        </product>
                    </items>
                </items>
            </getByIdReturn>
        </ns1:getByIdResponse>
    </soapenv:Body>
</soapenv:Envelope>

Wot a bloody mouthful!

PHP takes all that in its stride, and rehydrates that into an object:

object(stdClass)#2 (3) {
    ["account"]=>
    object(stdClass)#3 (5) {
        ["address"]=>
        object(stdClass)#4 (4) {
            ["country"]=>
            string(14) "United Kingdom"
            ["id"]=>
            float(1)
            ["localPart"]=>
            string(6) "London"
            ["postcode"]=>
            string(3) "E18"
        }
        ["dateOfBirth"]=>
        string(10) "1970-02-17"
        ["firstName"]=>
        string(4) "Adam"
        ["id"]=>
        float(2)
        ["lastName"]=>
        string(7) "Cameron"
    }
    ["id"]=>
    float(2011)
    ["items"]=>
    array(3) {
        [0]=>
        object(stdClass)#5 (4) {
            ["count"]=>
            float(15)
            ["id"]=>
            float(14)
            ["price"]=>
            float(16.17)
            ["product"]=>
            object(stdClass)#6 (3) {
                ["description"]=>
                string(7) "Penguin"
                ["id"]=>
                float(3)
                ["rrp"]=>
                float(4.56)
            }
        }
        [1]=>
        object(stdClass)#7 (4) {
            ["count"]=>
            float(19)
            ["id"]=>
            float(18)
            ["price"]=>
            float(20.21)
            ["product"]=>
            object(stdClass)#8 (3) {
                ["description"]=>
                string(8) "Pangolin"
                ["id"]=>
                float(7)
                ["rrp"]=>
                float(8.9)
            }
        }
        [2]=>
        object(stdClass)#9 (4) {
            ["count"]=>
            float(23)
            ["id"]=>
            float(22)
            ["price"]=>
            float(24.25)
            ["product"]=>
            object(stdClass)#10 (3) {
                ["description"]=>
                string(8) "Playtpus"
                ["id"]=>
                float(11)
                ["rrp"]=>
                float(12.13)
            }
        }
    }
}

That's cool, but if we look at the SOAP response, we do have the object information in there too. Here's an extract:

xsi:type="ns2:Invoice"&gt;
    &lt;account xsi:type="ns2:PersonalAccount"&gt;
        &lt;address xsi:type="ns2:Address"&gt;
            &lt;country xsi:type="xsd:string"&gt;United Kingdom&lt;/country&gt;
            &lt;id xsi:type="xsd:double"&gt;1.0&lt;/id&gt;
            &lt;localPart xsi:type="xsd:string"&gt;London&lt;/localPart&gt;
            &lt;postcode xsi:type="xsd:string"&gt;E18&lt;/postcode&gt;
        &lt;/address&gt;
        &lt;dateOfBirth xsi:type="xsd:string"&gt;1970-02-17&lt;/dateOfBirth&gt;
        &lt;firstName xsi:type="xsd:string"&gt;Adam&lt;/firstName&gt;
        &lt;id xsi:type="xsd:double"&gt;2.0&lt;/id&gt;
        &lt;lastName xsi:type="xsd:string"&gt;Cameron&lt;/lastName&gt;
    &lt;/account&gt;
    &lt;id xsi:type="xsd:double"&gt;2011.0&lt;/id&gt;
    &lt;items xmlns:soapenc="http://schemas.xmlsoap.org/soap/encoding/" soapenc:arrayType="xsd:anyType[3]" xsi:type="soapenc:Array"&gt;
        &lt;items xsi:type="ns2:InvoiceLine"&gt;
            &lt;count xsi:type="xsd:double"&gt;15.0&lt;/count&gt;
            &lt;id xsi:type="xsd:double"&gt;14.0&lt;/id&gt;
            &lt;price xsi:type="xsd:double"&gt;16.17&lt;/price&gt;
            &lt;product xsi:type="ns2:Product"&gt;
                &lt;description xsi:type="xsd:string"&gt;Penguin&lt;/description&gt;
                &lt;id xsi:type="xsd:double"&gt;3.0&lt;/id&gt;
                &lt;rrp xsi:type="xsd:double"&gt;4.56&lt;/rrp&gt;
            &lt;/product&gt;
        &lt;/items&gt;
        &lt;items xsi:type="ns2:InvoiceLine"&gt;
            &lt;!-- etc &gt;


Hidden away in there we see there's an Invoice, PersonalAccount, Address, InvoiceLines and Products.

We can leverage this information to point PHP at classes to use when rehydrating the response, which is cool. We give the SoapClient a type map:

$options = [
    'trace' => true,
    'typemap' => [[
        'type_ns' => 'http://accounts.adamcameron.me',
        'type_name' => 'Address',
        'from_xml' => ['\me\adamcameron\accounts\Address', 'createFromXml']
    ],[
        'type_ns' => 'http://accounts.adamcameron.me',
        'type_name' => 'Account',
        'from_xml' => ['\me\adamcameron\accounts\Account', 'createFromXml']
    ],[
        'type_ns' => 'http://accounts.adamcameron.me',
        'type_name' => 'Product',
        'from_xml' => ['\me\adamcameron\accounts\Product', 'createFromXml']
    ],[
        'type_ns' => 'http://accounts.adamcameron.me',
        'type_name' => 'InvoiceLine',
        'from_xml' => ['\me\adamcameron\accounts\InvoiceLine', 'createFromXml']
    ],[
        'type_ns' => 'http://accounts.adamcameron.me',
        'type_name' => 'Invoice',
        'from_xml' => ['\me\adamcameron\accounts\Invoice', 'createFromXml']
    ]]
];

$client = new \SoapClient($wsdl, $options);

This maps the SOAP type to a PHP class, and method to use to rehydrate the object.

Running the code again with the typemap yields a fully-modelled result:

object(me\adamcameron\accounts\Invoice)#3 (3) {
    // ...
    ["account"]=>
    object(me\adamcameron\accounts\Account)#5 (5) {
        // ...
        ["address"]=>
        object(me\adamcameron\accounts\Address)#9 (5) {
            // ...
        }
    }
    ["items"]=>
    array(3) {
        [0]=>
        object(me\adamcameron\accounts\InvoiceLine)#13 (4) {
            // ...
            ["product"]=>
            object(me\adamcameron\accounts\Product)#15 (3) {
                // ...
            }
            // ...
        }
        [1]=>
        object(me\adamcameron\accounts\InvoiceLine)#14 (4) {
            // ...
            ["product"]=>
            object(me\adamcameron\accounts\Product)#17 (3) {
                // ...
            }
            // ...
        }
        [2]=>
        object(me\adamcameron\accounts\InvoiceLine)#16 (4) {
            // ...

            ["product"]=>
            object(me\adamcameron\accounts\Product)#19 (3) {
                // ...
            }
            // ...
        }
    }
}

(I've elided the irrelevant stuff from that one).

So that's pretty handy.

I do wish I could provide the typemap with each call though if I wanted to, not just when creating the SoapClient object. Oh well: it was no huge hardship.

There's no trick on the class side of things at all. Here's the InvoiceLine class:

<?php

namespace me\adamcameron\accounts;

class InvoiceLine {

    public $id;
    public $product;
    public $count;
    public $price;
        
    function __construct(int $id, Product $product, int $count, float $price){
        $this->id = $id;
        $this->product = $product;
        $this->count = $count;
        $this->price = $price;
    }
    
    static function createFromXml ($xml){
        $sxe = is_string($xml) ? new \SimpleXMLElement($xml) : $xml;
        $obj = new InvoiceLine(
            (int)$sxe->id,
            Product::createFromXml($sxe->product),
            (int)$sxe->count,
            (float)$sxe->price
        );
        return $obj;            
    }
}


OK the one trick is that the SoapClient only passes the "top level" object's XML through to the top level method. So in this case SoapClient calls InvoiceLine->createFromXml as per the type map, but from there it's up to me to do the same for any other objects this one uses, for example a Product in this case.

Oh and the other trick is that because of this, I need that XML / string differentiator at the beginning of the method. SoapClient passes in a string, but once I've converted that to XML, that's what the other calls will receive (ie: Product::createFromXml receives XML, not a string). That took me a while to nut-out, but that's just me being a div.

Oh yeah, and don't forget to explicitly cast the values going into yer properties, otherwise you'll end up with XML in there, not the actual values. That threw me for a while as well :-/

All the rest of the PHP code for this is on GitHub too.

The problem I had with the Adobe web service is that it didn't bother including the type attributes in its responses, other than on the container element (so in my example the array of InvoiceItems would have had a type on it). This means I had to pass the entire collection container into a handler method which then read the XML tag names and hand-cranked a switch based on that as to what rehydration method to use. I'm gonna try to contrive a non-business-sensitive version of that to show how I dealt with it.

Now... I've received orders from a mate that I'm expected at the pub in an hour. So I had better get cracking. Sorry to make you think about SOAP. And Adobe.

Righto.

--
Adam

Saturday 17 December 2016

That new Star Wars movie

G'day:
If you need to be told that an article entitled "That new Star Wars movie" is perhaps going to discuss that new Star Wars movie, and intrinsically that's going to include details of the plot then... well here you go:

This contains spoilers.

I'll leave some space before I say anything else, in case that's not clear enough for you, and you think "oh, I hope what Cameron says here doesn't discuss that new Star Wars movie... I haven't seen it and I don't want it spoiled".































Oh for fuck's sake why do I do this to myself?

Firstly, Adam Tuttle is dead right on two counts:


This is true. I watched the trailers for this and though: "haha, nicely played: this doesn't look like crap!", and pretty much decided to see it on that basis. He followed up with this:


I'm the same. I've not actually enjoyed one of these movies since the third one. To save some confusion: I don't give a shit how Lucas decided to number these, or whether they're prequels, sequels or spin offs: I number them chronologically based on their release dates. In this case I mean: Return of the Jedi: the third one. I liked that one. I was 13 when I saw it (in 1983), and I've always had a soft spot for "space ships and laser guns" movies. And it was a good kids' movie. And I was a kid.

It demonstrates AdamT's point that I also knew what date this new one was being released (well: within a week or so of the date: "early Dec"), and I figured a coupla nights ago... I'd be in Galway on Saturday afternoon with two options:
  1. sit at the pub and write a blog article about consuming SOAP web services with PHP. Oh, and drink Guinness;
  2. or go to the local cinema and kill a coupla hours watching this movie first.
I don't wanna think about SOAP yet, so I decided on the latter. On the way home from the cinema I decided to procrastinate on the SOAP thing further by wittering on about the movie instead. I've got the Guinness though. There's always Guinness.

But anyway AdamT was right... there was some anticipation from me to get to see this latest Star Wars movie, and given - despite my best efforts - I don't actually enjoy them, I suspect it's just the nostalgia thing he mentioned.

So, yeah, I scooted down to the cinema and watched the movie. Interestingly there was only about another dozen people in the auditorium. I guess it's either not the draw "a Star Wars movie" used to have, or Saturday afternoon is not a popular time, or the Irish are too sensible to waste their time on such nonsense. I like to think it's the latter.

In case you don't know, this one is the story about how the goodies got hold of the Death Star plans just before the beginning of the first movie, and got them to Princess Leia and R2D2 and what not, and off they went to be chased by that Star Destroyer, and the rest of Star Wars goes from there.

Here's the problem with this new one from the outset: we already know the goodies in this movie succeed in their efforts to steal the plans, and we also know that they all die. Hey I told you there was going to be spoilers. Why did we know they'd all be dead at the end of this one? Well if they weren't they'd still be around for Star Wars etc, wouldn't they?

As a sidebar I am a big fan of the movie Alien, and before I developed a sense for decent movie writing, I also really liked Aliens. I always thought it'd be way cool if there was a linking story covering the period at the LV426 colony before they all got wiped out (some scenes of this made it into the extended mix of Aliens)... showing the colonists being overwhelmed, and only two of them being alive at the end. But even then it occurred to me the denouement wouldn't work as we already know what it would be.

Same here with this movie: Felicity Jones (I've no idea what her character name was: it didn't matter) was always gonna end up dead. And that other geezer she was with. Dead. Along with all the red shirts they were with. Well obviously they were gonna end up dead: they were only making up the numbers (this includes the yeah-we-get-it-it's-The-Force blind dude and his... brother? Pal? Who knows? Who cares?). This was, accordingly, a movie without any real overarching sense of drama.

One thing I did like about this movie was all the nods to the earlier movies there were. I'd usually think this would be self-indulgement / self-knowing / "Joss-Whedon-esque" sort of movie making, but hey, this thing is a nostalgia exercise more than anything else, so why not. I mention this cos I chuckled when they did indeed open with a triangular thing coming down from the top of the screen - eg: the Star Destroyer in the first movie - but this time it wasn't some big spaceship, it was a visual illusion of the way a planet's rings were in the planet's shadow (you'll need to see the scene to get what I mean). On the whole these things were inconsequential but I spotted a few, and they made me chuckle.

I was surprised to see Mads Mikkelsen in this (slumming it slightly, IMO), and he delivered his lines well and seemed convincing in his role... although it was a pretty small if pivotal one. I mention this because most of the rest of the acting was either pretty bland, or the players were the victim of pretty turgid writing (I was reminded of Harrison Ford's quote "George, you can type this shit, but you can't say it!". This was alive and well in this movie too, despite Lucas having nothing to do with it).

Forest Whitaker was a prime example of victimised actor here. His lines were so awful even he couldn't save them. I actually wonder if there was more material for his character originally which was excised, cos I really don't see why he was in the movie.

Felicity Jones was OK, but in comparing her to... ooh... [thinks]... Daisy Ridley in the preceding movie, who I though "hey, you've made a good character here!", I didn't get the same reaction. But she was completely OK, and one of the few people not over-egging their performances.

The comedy robot sidekick was better than usual, but it was still a comedy robot sidekick. It's interesting though that - on reflection - it was probably the second-most-rounded-out character after Jones's one. More so than all the other humans.

On the whole the script was dire. Uncharacteristically for me I found myself repeatedly muttered "for fuck's sake: really? Did you really just make the poor actor utter that line?" But I have to realise it's aimed at people with limited attention spans, and limited... well... age. Be that chronologically or... well... ahem.

What's with the capes? Why did that dude... the baddy guy who wasn't Tarkin or Darth Vader... wear a uniform which otherwise would never have a cape (none of the other dudes with the same uniform had one), had a cape basically clipped on to it. Who the fuck has worn a cape since the Edwardian era?? I never understood that about the likes of Batman or Superman either. Fucking daft.

But actually that baddy guy wasn't too badly drawn and portrayed either. He didn't seem "one note evil" like CGI Cushing or Darth Vader. Vader with his cape. Fuckin' dick.

(later update: shit it was Ben Mendelsohn. Fair enough he did a decent job then. Thankfully his script wasn't as bad as Whitaker's)

The visual composition of the thing was impressive, as one would expect from one of these movies. I think they overdid the "huge impressive but strangely odd design for the given situation" buildings a bit. And it was clear there was a design session of "we need new environments... these things always have new environments... I know: rain! Let's do rain! We've not done rain before! Oh and Fiji too. Let's make one of the planets look like Fiji: we've not done that before either". Still: it all looked impressive. It also pretty much looked real too, which is an improvement on some of its predecessors.

There was also a lot of "exciting" action set-pieces, except for the fact they're not exciting at all, because we all know that the action bits during the body of  the movie will only ever serve to maintain the attention-span of the viewers between sections of exposition or travel to the next set piece, and nothing really important will happen during them. Like key characters being killed or anything. Just the red shirts (or white-suited storm-troopers in this case).

All I could think in the final space battle thing was that - once again - they've committed too many resources to this thing: there's not a few TIE Fighters, there's a bloody million of them, so obviously any of that action is not going to actually contribute to the plot, as there's no way the goodies can realistically beat them ship to ship, so something else needs to happen. So: no drama, and might as well not bother. It doesn't even look impressive as lots of small things whizzing around the place is not impressive. So it's just a matter of sitting there going "oh just get on with it, FFS. Get back to the plot".

I also thought the people in the Star Wars technical design dept should talk to the ones from the Battlestar Galactica one. Those were capital ships. None of this "we'll fire a coupla laser beams at you every few seconds", but "we'll throw up a wall of lead and fire, and small craft just ain't getting through it". Star Wars capital ships just aren't impressive. Oh, OK, I did like the way they finally got rid of the shield gateway thingey though. That was cool. It also reminded me of the scene in RotJ when one of the star destroyers lost control, and crashed into the surface of the Death Star.

Speaking of which: why was there no mention of the fact they were building two Death Stars? Those things would take ten years to build, so they were clearly both underway at the same time.

I did think "ooh shit, now yer fucked" when the AT-ATs showed up at the beach battle. Although obviously they were just gonna get destroyed or just not matter anyhow. I grant some of the ways they were destroyed struck a chord, especially in contrast to their seemingly imperviousness (is that a word?) in The Empire Strikes Back. The rendering of them made them look solid and foreboding though. Completely impractical, but quite foreboding.

How come only one person in the movie had a fully-automatic weapon? And why did it have a slide action (like a pump-action shotgun) which occasionally needed using?

What was with that computer of theirs? At the end when they were trying to get the plans. why did it need a manually controlled thingey to find the hard drive that they were looking for? I realise it was a plot device to slow things down a bit so the baddie could get there for that final showdown, but is that the best they could come up with? Shitty, lazy-arse writing.

Why was the controller console for the satellite dish way out there at the end of that catwalk?

Why did the Death Star miss from that range? I mean other than a setup so that the two goodies weren't instantly vaporised, instead giving them a moment to be reunited and have a wee hug (that was telegraphed too. Sigh) before being all tsunami-ed.

Why the fuck do I keep going to these movies?

In the end, I'd rate this movie as follows:

  • visually impressive in a vapid way;
  • not bad for a Star Wars movie. Probably the "best" one since RotJ;
  • but let's make it clear: that's damning it with faint praise. This is an intellectually barren movie, aimed at kids (at least psychologically, if not chronologically). In that I know adults that actually like this shit, it's just further proof of the infantilisation of our culture, and at that I despair.
  • I'll  give it 6/10 mostly cos it does indeed achieve what it sets out to do... I'm just not the right audience for it. But seeing the kiddies waiting outside for the next session all excited made me remember what it was like when I first went to Star Wars, aged eight, 38 years ago.
  • If I was a kiddie, it'd be an 8/10, I reckon. It's a bit bleak for a kiddie though, as they won't understand all the rest of the story kicks-off from the end of this one. That and that pretty much everyone of note in the movie dies. But at least they wouldn't notice how fucking stupid and bad almost all of the human-element of the movie was.

Oh... I thought the very ending was good: getting the plans to Leia's spaceship and off they went... 5min later for the Star Wars plot to start. That was all right.

Right. Another Guinness and I better try to find 1000-odd words to write about SOAP.

Sorry for the off-topic shite, but writing this all down here will save me some Twitter conversations about it. And, hey, it's possible click bait ;-)

Righto.

--
Adam

Friday 16 December 2016

PHP: adolescent weirdness with arrays

G'day:
This was put on my radar the other day, by Alexander Lisachenko:


Well put. The code concerned is thus:

<?php
$A = [1 => 1, 2 => 0, 3 => 1];
$B = [1 => 1, 3 => 0, 2 => 1];

var_dump($A > $B);
var_dump($B > $A);


var_dump($A < $B);
var_dump($B < $A);

And it outputs...

C:\temp>php array.php
bool(true)
bool(true)
bool(true)
bool(true)

C:\temp>

Well that's quite... contrary.

PHP Team member Kalle Sommer Nielsen explained that this sort of comparison (between arrays) is "unsupported" in PHP.

Further conversation ensued, which you can read on Twitter, rather than me repeating it here.

My thoughts on this are that it's completely fine for PHP to not support this sort of comparison, but in that case the way it should deal with it is actively not support it: raise an IllegalOperationException or some such (some extension of LogicException, anyhow). It should not just issue forth illogical and misleading results. Kalle explains some of the background to the situation, which was helpful.

Another issue I have is that if we conclude that asking "is this array greater than that array" doesn't simply result in an error, and it must answer "true" or "false", then the answer ought to be false. Kalle contended it didn't matter, but my parallel is "cheese is greater than blue (true or false)?". It's a nonsensical question, But the answer is not "true".

The conclusion was: well it's done now... one shouldn't try to do these things, and whilst PHP might not handle it in an ideal fashion, it's a sufficient edge-case to not warrant remedial action. All fair enough.

I had a quick look at what PHP does with various comparisons:

<?php

$i1 = 5;
$i2 = 7;
echo "$i1 > $i2: " . booleanAsString($i1 > $i2) . PHP_EOL;
echo "$i1 < $i2: " . booleanAsString($i1 < $i2) . PHP_EOL;

$f1 = 5.5;
$f2 = 7.7;
echo "$f1 > $f2: " . booleanAsString($f1 > $f2) . PHP_EOL;
echo "$f1 < $f2: " . booleanAsString($f1 < $f2) . PHP_EOL;

$s1 = "a";
$s2 = "z";
echo "$s1 > $s2: " . booleanAsString($s1 > $s2) . PHP_EOL;
echo "$s1 < $s2: " . booleanAsString($s1 < $s2) . PHP_EOL;

$s1 = "A";
$s2 = "a";
echo "$s1 > $s2: " . booleanAsString($s1 > $s2) . PHP_EOL;
echo "$s1 < $s2: " . booleanAsString($s1 < $s2) . PHP_EOL;

$b1 = true;
$b2 = false;
echo sprintf("%s > %s: ", booleanAsString($b1), booleanAsString($b2)) . booleanAsString($b1 > $b2) . PHP_EOL;
echo sprintf("%s < %s: ", booleanAsString($b1), booleanAsString($b2)) . booleanAsString($b1 < $b2) . PHP_EOL;

$o1 = (object) ["i"=>5];
$o2 = (object) ["i"=>7];
echo sprintf("%s > %s: ", json_encode($o1), json_encode($o2)) . booleanAsString($o1 > $o2) . PHP_EOL;
echo sprintf("%s < %s: ", json_encode($o1), json_encode($o2)) . booleanAsString($o1 < $o2) . PHP_EOL;

$a1 = ["i"=>5];
$a2 = ["i"=>7];
echo sprintf("%s > %s: ", json_encode($a1), json_encode($a2)) . booleanAsString($a1 > $a2) . PHP_EOL;
echo sprintf("%s < %s: ", json_encode($a1), json_encode($a2)) . booleanAsString($a1 < $a2) . PHP_EOL;


$a1 = [5];
$a2 = [7];
echo sprintf("%s > %s: ", json_encode($a1), json_encode($a2)) . booleanAsString($a1 > $a2) . PHP_EOL;
echo sprintf("%s < %s: ", json_encode($a1), json_encode($a2)) . booleanAsString($a1 < $a2) . PHP_EOL;

$a1 = [5=>5];
$a2 = [7=>7];
echo sprintf("%s > %s: ", json_encode($a1), json_encode($a2)) . booleanAsString($a1 > $a2) . PHP_EOL;
echo sprintf("%s < %s: ", json_encode($a1), json_encode($a2)) . booleanAsString($a1 < $a2) . PHP_EOL;

$a1 = [1 => 1, 2 => 0, 3 => 1];
$a2 = [1 => 1, 3 => 0, 2 => 1];
echo sprintf("%s > %s: ", json_encode($a1), json_encode($a2)) . booleanAsString($a1 > $a2) . PHP_EOL;
echo sprintf("%s < %s: ", json_encode($a1), json_encode($a2)) . booleanAsString($a1 < $a2) . PHP_EOL;

$a1 = [1 => 1, 2 => 0, 3 => 1];
$a2 = [1 => 1, 3 => 1, 2 => 0];
echo sprintf("%s > %s: ", json_encode($a1), json_encode($a2)) . booleanAsString($a1 > $a2) . PHP_EOL;
echo sprintf("%s < %s: ", json_encode($a1), json_encode($a2)) . booleanAsString($a1 < $a2) . PHP_EOL;


function booleanAsString($expression){
    return $expression ? "true" : "false";
}

I don't think any of that lot needs explaining: it's straight forward enough. There's a few baseline control expressions which apply the operator to numeric operands, which is clearly gonna have predictable results. Then I check with strings (including case differences), booleans, and then arrays (indexed and associative). The output:

5 > 7: false
5 < 7: true
5.5 > 7.7: false
5.5 < 7.7: true
a > z: false
a < z: true
A > a: false
A < a: true
true > false: true
true < false: false
{"i":5} > {"i":7}: false
{"i":5} < {"i":7}: true
{"i":5} > {"i":7}: false
{"i":5} < {"i":7}: true
[5] > [7]: false
[5] < [7]: true
{"5":5} > {"7":7}: false
{"5":5} < {"7":7}: false
{"1":1,"2":0,"3":1} > {"1":1,"3":0,"2":1}: true
{"1":1,"2":0,"3":1} < {"1":1,"3":0,"2":1}: true
{"1":1,"2":0,"3":1} > {"1":1,"3":1,"2":0}: false
{"1":1,"2":0,"3":1} < {"1":1,"3":1,"2":0}: false

Things work reasonably sensibly until we get to associative arrays, at which point PHP starts spouting nonsense.

I had a look around for something in the docs saying "don't do this", but what I did find kinda indicates there might indeed be a problem here (at least in the docs, if not in PHP's behaviour).

I've extracted this from "Comparison with Various Types":


Array with fewer members is smaller, if key from operand 1 is not found in operand 2 then arrays are uncomparable, otherwise - compare value by value (see following example)
(obviously that's my emphasis)

But this isn't so. See this example:

$a1 = ["a" => 1, "b" => 0, "c" => 1];
$a2 = ["a" => 1, "c" => 0, "b" => 1];
echo sprintf("%s > %s: ", json_encode($a1), json_encode($a2)) . booleanAsString($a1 > $a2) . PHP_EOL;
echo sprintf("%s < %s: ", json_encode($a1), json_encode($a2)) . booleanAsString($a1 < $a2) . PHP_EOL;

All the keys are present in both: "a", "b", "c", but the comparison doesn't work. I suspect it's the case of  poor wording in the docs, and it's down to whether the keys are in the same order too.

Also I'm not sure that suggesting they're "uncomparable" is helpful wording here. To me that indicates one would expect an exception if one tried. It should say "then the result is unreliable, and the operation should not be used".

I also note there's a big, highlighted warning about comparing floats:



Perhaps the same treatment should be given to array comparisons. If it tripped-up Mr Lisachenko - who seems to be pretty bloody good & experienced PHP dev - then it's gonna trip-up a lot of people.

Bottom line I agree that the handling here is "unfortunate", but it is pretty predictable given how PHP tends to handle not-obvious things, so one kinda oughta expect it. Equally it could be remediated by a documentation improvement, and I don't think it's worth anyone putting any effort into to "fixing" it.

Righto.

--
Adam

cf(it's a~)live.net

G'day:
I received some excellent news from Russ Michaels this morning:

cflive.net is now back, thanks to hostek who donated me a server for cflive and cfmldeveloper, which will also be back soon.

Cool! I missed cflive.net. It might not be as shiny and fully-featured as trycf.com, but it has the one benefit of just getting on with it. It saves yer code and runs it. trycf.com monkeys with both the code and the output thereof, which I've never liked, and never felt was that reliable for anything other than superficial demonstrations. I have always felt happy testing issues on cflive.net, whereas I find trycf.com to be a bit of an "unreliable narrator".

The one feature I wish cflive.net would pinch from trycf.com is the ability to save code with a shareable link.

Anyway... Both services are valuable - almost indispensable - resources for the CFML community, and I'm glad cflive.net is back.

Good stuff, Russ; and thanks, Hostek, on behalf of the CFML community.

Righto.

--
Adam

Wednesday 14 December 2016

Code Twats and Code Bullies

G'day:
This is something I should have perhaps written about a while back when it first got on my radar, but was never sure how to make a decent-sized article of, so didn't bother. However the situation has arisen again, and I figure it's probably worth raising, even if it's only short.

(ed: having finished it now, I don't think it's my finest-ever article, but... oh well).

A year or so ago we had a new dev who was reasonably good at PHP (certainly better then me, to damn them with faint praise), and not a bad programmer. However they had:

  • very little experience in working within a team,
  • very little experience with adhering to development standards other than their own,
  • and very little experience with anyone making observations about their work.
To be honest, I rate those three points as more important than technically how skilled a person might be at the general vocab and grammar of a language.

We do rigorous code reviews at our joint, and on the whole the team members enjoy having eyeballs on their code as we all "get" that code is not "mine" or "yours", it's "ours", so we all need to be happy with it. We all have to have input into it, and to employ a tired cliche or two: two heads are better than one, and the same applies to "more eyes". Code review will reveal trivial stuff like unused or unVARed variables, typos and coding standard breaks; more significant issues like sub-par or expensive logic, missing test coverage; through to more theoretical considerations such as misuse (or lack of use ~) of design patterns.

A code review will also throw up more subjective stuff like "WTF is going on here?" This can be for a number of reasons:

  • It could be because the reviewer is unaware of the context or the requirement of the code,
  • they might not be aware of a technique,
  • or the code might simply be:
    • obfuscated,
    • obtuse,
    • less than ideal in design,
    • or simply wrong.

We all write code that falls into one or more of these latter categories: shit happens. The thing is that code is collectively owned, so it kinda needs to regress to the mean slightly. "Smart" code ain't smart if they team can't maintain it, or it doesn't follow precedents already set in the code, or doesn't follow coding standard or other various "code collective" considerations. Having code that the team can maintain after it's written is the most important thing about collectively-own code.

So... code needs reviewing. That's a given. If yer not doing formal code review at your gig: you are, IMO, not conducting yerself professionally. If you work by yourself it's even more critical you find a way to get more eyes on yer code, cos working by yerself you're just in a locked-in cycle of kinda code inbreeding (if you'll permit that).

Another thing that less-experienced devs will do is to write code to feed their ego because they think they are clever. Now I use "less-experienced" in a measured way here. I don't mean "less experienced in coding", I mean "less-experienced in working as a team, and less-experienced at realising code is not all about me me me". And less experienced in understanding "no-one gives a shit how clever you think you are". There was a well-realised dev stereotype going around a decade or so ago - and seems to have died off these more enlightened days - of "Code Ninja". There was no such thing: there was just "Code Twats". You're not a "ninja" (FFS!) if the people you're coding with are going "um... WTF?" You're just a Code Twat.

If we combine people who aren't good team players, code almost as an act of hubris, and generally aren't quite so clever as they've lead themselves to believe, then code reviews can get... awkward.

Too many times with non-team-players (Code Twats) I've seen code reviews in which a team member has flagged up some "strategically challenging" (ahem) code and detailed why they thought there was a preferable approach.  And the code's author just wouldn't have a bar of it. They liked their code, and had already decided they were "right", and anyone else's consideration was just mistaken basically cos they weren't clever enough to understand it. So refused to heed the review. More of the code reviewers pitched in, and said "well actually the review's kinda right: the code's not clear, and not the way we do things, even if it's syntactically correct and works". Nope. The author just got more defensive, started googling justifications of why they were right (because, after all, being the "victim" of a code review is an exercise in being right, right? Right?!). And the responses were getting longer and longer. And I suspect typed with heavier and heavier and stabbier and stabbier fingers.

The code review went around the houses for over 100 comments. There was about ten times more text in the comments than there was in the code under review. This is an egregious example, but it sticks in my mind. Also, btw, was not the only review that ended up that way with this individual. Who has thankfully left the company now. There's been similar reviews with other bods too, to a slightly lesser degree. It's not uncommon.

Ultimately the team lead stepped up and gave everyone a ballocking. Although I think perhaps the person under review shoulda been the only person on the receiving-end of said ballocking,because it was clear very quickly the team wasn't happy with the code, therefore the code needed rework. End-of. It's not their fault than the author wasn't having a bar of it. The one point the lead did have is that a couple of us (possibly mostly me) are supposed to be senior, so shouldn't've been so self-indulgent as to perpetuate the review cycle. We did, after all, have our own work to do. That said we were "senior" without any actual degree of seniority over the dev concerned, so we couldn't exactly go "Just. Do. What. Yer. Told" (which is all that needed to be said in this case, realistically). That was for the lead to do. And they shoulda done it earlier. (Said lead has also since left the company, but I look forward to the pint I am having with them this evening ;-)

What didn't occur to me until yesterday as I was reading more about code review failures / challenges, is that these devs who simply won't join the team and perpetuate these seemingly endless cycles of code review back and forth are actually "Code Bullies". It's just bullying. They're badgering their reviewers into submission, hoping they will just go "oh FFS, I don't care enough about this, OK have it your way". I have seen this happen. Hell, I have to admit I've engaged in it! (not my finest hour, and I've tried to learn from this, and I don't do it any more). The problem is I see the artefacts of these bullying tactics in our code every day. Bullying isn't just a temporally-limited thing; its impact has long-reaching consequences. Its victims continue to suffer (hey, if you were looking at some of this code, you'd not be thinking "suffering" was an overstatement ;-).

This situation needs knocking on the head. There's perhaps some guidelines which can help mitigate this:

  • firstly the dev needs to actually consider the bigger picture, and it's not a competition, and no-one cares if they're "right" according to only a subset of considerations. This is difficult to control, but if it becomes symptomatic, it needs proactive attention.
  • If the exchange between the dev and the reviewer goes into the third round, then they need to stop being keyboard warriors, and talk to each other. Odds-on they're sitting next to each other, or on the same campus. If they're not on the same campus, then there's this thing called a telephone. use it. Or, hey, Skype or something more modern than that. But switch to "interactive mode".
  • If more than one reviewer are in accord: they're right and the dev is wrong. Perhaps not completely wrong, but the work needs rework anyhow. But there's a very good chance the dev is outright "wrong" according to more criteria than they might be "right" (which is generally just down to code twattery anyhow). Reject the pull request (or some equivalent thereof; end the code review anyhow) as a first step here.
  • Make sure never to employ a passive-aggressive, sarcastic, supercilious or dismissive tones in your code review comments. You're just being a prick. Being a prick and then covering yerself by putting a winky at the end of the comment doesn't mean you're not a prick. It just demonstrates you know yer a prick, and act that way anyhow. Thus making you even more of a prick. This applies to both dev and reviewer, btw.
  • If there's more than say ten comments on one code review point: the lead needs to step up. This ought to be either cos they're already watching the code review, or one of the participants raises it with them (which they should be completely prepared to do. That's what the lead is for!). Then the lead acts as sole arbiter, and whatever they say gets done. The first step is rejecting the pull request, if the decision is that there's rework to do.
  • Most importantly: code bullies need to be admonished for their behaviour. Like I mean officially admonished, like any other sort of office bullying. It's just not bloody on. They are deleterious to morale, and they infect the codebase with their blight.

We can all do without this sort of anti-social (not to mention unprofessional) behaviour in our work place. Don't be part of the feedback loop, don't be a Code Twat, and seriously... call out Code Bullies when you see them.

Righto.

--
Adam

Saturday 3 December 2016

Thoughts and implementation on that daft FizzBuzz interview question

G'day:
I'm not quite sure where I'm gonna go with this article yet. Usually when I start an article like this it ends up being a bit rubbish, so... well sorry if it turns out this way this time too.

Right so there was a conversation on the CFML Slack channel last week stemming from someone deciding to use the old FizzBuzz question as part of a job interview. I'm not a great proponent of such things in interviews, but then back when I used to conduct such things I was shit at doing 'em anyhow, so what do I know?

But I don't think it's a sensible measure of anything to make someone code something in the actual interview: interviews can be high-stress environments, with restricted time-framing, and I don't think that reflects the general programming environment. Well not one I'd want to work in, anyhow. I've once been asked to design a database - in an interview for a developer role, I hasten to add - I just said "no, I'm not going to do that; I don't think that's a good use of anyone's time, and it casts you in a bad light for asking, and me in a bad light as I fumble around trying to do something that might normally take hours in only a few min. So: no" (that's a paraphrase, but it is the general gist of how the conversation went). I think perhaps had I actually wanted the job I might have humoured them. Maybe. It's dumb-arse anyhow.

Fortunately in this fizzbuzz situation the person conducting the interview was like-minded, and told the candidate to take it home with them and flick something back later. That's reasonable.

But what should one look for in a situation like this? And what should one deliver as a solution?

Firstly... in case you are like me and had no idea what people were on about when they started talking about FizzBuzz, this is the question:

Write a program that prints the numbers from 1 to 100. But for multiples of three print "Fizz" instead of the number and for the multiples of five print "Buzz". For numbers which are multiples of both three and five print "FizzBuzz".
(from http://wiki.c2.com/?FizzBuzzTest)

The Slack channel got to messing around, and as one can probably imagine, shortly people starting answering the question. A typical answer might be like this:

for ($i=1; $i <=100; $i++) {
    $s = "";
    if ($i % 3 == 0) $s .= "fizz";
    if ($i % 5 == 0) $s .= "buzz";
    
    echo $s ?: $i;
}

That's simple and to the point, but I would not be impressed with that answer. Why? It's not reusable at all. OK, cool, the question didn't ask for re-use. That being the case, why bother with the code? Just use the output:

echo "12fizz4buzzfizz78fizzbuzz[etc]buzz";

If yer not needing a reusable answer... don't piss about writing code to do it. Just answer it. If I was in the position to discuss this with the interviewer - rather than just submit the answer - I would indeed consider starting with that answer there. Not to be a smart arse, but to let them know I think about that sort of thing.

But I'd then go on to point out some flaws in the question and in the answer. I think it's reasonable to provide slightly reusable code, not least of all so one can test it. One could test a file that simply has that statement in it, but it's not so orthodox to do so.

I put the answer in a function:

function getFirst100FizzBuzzNumbers(){
    return "12fizz4buzzfizz78fizzbuzz[etc]buzz";
}

Then quickly realise that's still not very testable as testing frameworks usually expect code to be in classes. So I decide to do that: put it in a class.

However another thing I'd be wanting to do in this exercise is to demonstrate "test first" as a developmental approach. For this I just decided to use PHPSpec for a change today, as I heard someone mention it and thought "yeah, all right...". To get that strummed-up I need a composer.json file:

{
    "name" : "fizzBuzzChallenge",
    "require-dev" : {
        "phpspec/phpspec" : "^3.0"
    },
    "config": {
        "bin-dir": "bin"
    },
    "autoload": {
        "psr-4" : {
            "me\\adamcameron\\fizzBuzzChallenge\\" : "src/"
        }
    }
}

(some of that's from the PHPSpec docs, the rest is just "well I'll need to put the code somewhere, so might as well PSR-4 it and namespace it etc too).

Following the PHPSpec docs still, I start by describing my spec:


C:\src\php\php.local\src\puzzle\fizzbuzz>bin\phpspec desc me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle
Specification for me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle created in C:\src\php\php.local\src\puzzle\fizzbuzz\spec\FizzBuzzPuzzleSpec.php.


C:\src\php\php.local\src\puzzle\fizzbuzz>bin\phpspec run
me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle
  11  - it is initializable
      class me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle does not exist.

                                      100%                                       1
1 specs
1 example (1 broken)
25ms


  Do you want me to create `me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle`
  for you?
                                                                         [Y/n]

Class me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle created in C:\src\php\php.local\src\puzzle\fizzbuzz\src\FizzBuzzPuzzle.php.

                                      100%                                       1
1 specs
1 example (1 passed)
27ms


C:\src\php\php.local\src\puzzle\fizzbuzz>

So that's all quite cool. Notice that it created the spec class for me, and it also created the actual class for me too.

Now I have this:

namespace spec\me\adamcameron\fizzBuzzChallenge;

use me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle;
use PhpSpec\ObjectBehavior;
use Prophecy\Argument;

class FizzBuzzPuzzleSpec extends ObjectBehavior
{
    function it_is_initializable()
    {
        $this->shouldHaveType(FizzBuzzPuzzle::class);
    }
}


namespace me\adamcameron\fizzBuzzChallenge;

class FizzBuzzPuzzle
{
}

Now I need to spec-out my function (test first remember!):

class FizzBuzzPuzzleSpec extends ObjectBehavior {

    function its_getFirst100FizzBuzzNumbersMethodReturnsTheExpectedValue() {
        $this->getFirst100FizzBuzzNumbers()->shouldBe("12fizz4buzzfizz78fizzbuzz[etc]buzz");
    }
}


(It's not part of my requirement to spec-out the return type of the class, so I got rid of that one).

C:\src\php\php.local\src\puzzle\fizzbuzz>bin\phpspec run
me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle
  10  - its getFirst100FizzBuzzNumbersMethodReturnsTheExpectedValue
      method me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle::getFirst100FizzBuzzNumbers not found.

                                      100%                                       1
1 specs
1 example (1 broken)
227ms


  Do you want me to create
  `me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle::getFirst100FizzBuzzNumbers
  ()` for you?
                                                                         [Y/n]

  Method me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle::getFirst100FizzBuzzNumbers() has been created.

me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle
  10  - its getFirst100FizzBuzzNumbersMethodReturnsTheExpectedValue
      expected "12fizz4buzzfizz78fizzbuzz...", but got null.

                                      100%                                       1
1 specs
1 example (1 failed)
29ms


C:\src\php\php.local\src\puzzle\fizzbuzz>

Now I have the method skeleton as well:

public function getFirst100FizzBuzzNumbers()
{
    // TODO: write logic here
}


Which I replace with my implementation:

function getFirst100FizzBuzzNumbers(){
    return "12fizz4buzzfizz78fizzbuzz[etc]buzz";
}

Which works:

C:\src\php\php.local\src\puzzle\fizzbuzz>bin\phpspec run
                                      100%                                       1
1 specs
1 example (1 passed)
31ms


C:\src\php\php.local\src\puzzle\fizzbuzz>


However it's perhaps realistic to expect this thing to actually do the work dynamically instead of just returning a hard-coded string. So I shift my specification slightly to be "a function which takes an integer i and returns the first i FizzBuzz numbers.

The simplest spec for this is to just test that it returns a FizzBuzz number. The first FizzBuzz number is 1:

function its_getFizzBuzzNumbersMethodReturnsAFizzBuzzNumber() {
    $this->getFizzBuzzNumbers(1)->shouldBe(1);
}

Again I run the tests, and again it creates my method skeleton for me:

public function getFizzBuzzNumbers($argument1)
{
    // TODO: write logic here
}


Which I replace with enough code to pass.

public function getFizzBuzzNumbers($i) {
    return $i;
}


Now I'm intending to write enough of the algorithm to get me to 3, which is the first variation of the return value. I'm also gonna change the specification to return an array not a string. This is a nod to not assuming the use of the result, and an array is a better data type for a sequence of things. Coupled with this, I want to separate out the collection (the array) from the contents of the collection (FizzBuzz numbers). So I decide to spec a different function for now: getFizzBuzzNumber.

I create a spec:

function its_getFizzBuzzNumberMethodReturnsAFizzBuzzNumber() {
    $this->getFizzBuzzNumber(1)->shouldMatch('/\d+|fizz/');
}


And create a naive implementation of that:

public function getFizzBuzzNumber($i) {
    return $i;
}


And test the spec:

  16  - its getFizzBuzzNumberMethodReturnsAFizzBuzzNumber
      no match([array:1]) matcher found for [integer:1].

Erm... damn. What? This took a while to work out. PHPSpec seems to have forgotten that PHP is a dynamic, loosely-typed language, and seems to type-check the value before testing the pattern. If I returned a string, this spec worked:

public function getFizzBuzzNumber($i) {
    return (string) $i;
}

1 specs
1 example (1 passed)

Now I almost lived with that. I almost lived with returning the numeric FizzBuzz numbers as strings to make my pattern work. But that seemed arse-about-face. The spec was getting it "wrong", not my code. I chatted on the BDDLondon Slack Channel about how to test for "this OR that" (if not able to use a regex pattern"), and the answer was "cannae". This also had me thinking "I actually really hate methods that return two different types of thing, as it seems a bit messy. Then it occurred to me... it's not returning two different types of thing. It was returning one type of thing: it was returning a FizzBuzzNumber object. Aha! I was not after a collection of a mix of integers and strings "fizz", "buzz" or "fizzbuzz". They were all just FizzBuzzNumber objects. Cool.

So I spec-ed out a second class: FizzBuzzNumber. I'll spare you the stepped spec->code->repeat, and here's the end result. Spec first:

<?php

namespace spec\me\adamcameron\fizzBuzzChallenge;

use me\adamcameron\fizzBuzzChallenge\FizzBuzzNumber;
use PhpSpec\ObjectBehavior;
use Prophecy\Argument;

class FizzBuzzNumberSpec extends ObjectBehavior
{
    function it_isInitializable() {
        $this->beConstructedWith(1);
        $this->shouldHaveType(FizzBuzzNumber::class);
    }

    function it_isInitializableWithFizz() {
        $this->beConstructedWith("Fizz");
        $this->shouldHaveType(FizzBuzzNumber::class);
    }

    function it_isInitializableWithBuzz() {
        $this->beConstructedWith("Buzz");
        $this->shouldHaveType(FizzBuzzNumber::class);
    }

    function it_isInitializableWithFizzBuzz() {
        $this->beConstructedWith("FizzBuzz");
        $this->shouldHaveType(FizzBuzzNumber::class);
    }

    function it_isInitializableWithOtherInteger() {
        $this->beConstructedWith(2);
        $this->shouldHaveType(FizzBuzzNumber::class);
    }

    function it_isNotInitializableWithMultipleOfThree() {
        $this->beConstructedWith(6);
        $this->shouldThrow('\InvalidArgumentException')->duringInstantiation();
    }

    function it_isNotInitializableWithMultipleOfFive() {
        $this->beConstructedWith(10);
        $this->shouldThrow('\InvalidArgumentException')->duringInstantiation();
    }

    function it_returnsFizzWhenCreatingFromIntegerWithMultipleOfThree() {
        $this->beConstructedWith(1);
        $this::fromInteger(12)->shouldHaveType(FizzBuzzNumber::class);
        $this::fromInteger(12)->__toString()->shouldBe("fizz");
    }

    function it_returnsBuzzWhenCreatingFromIntegerWithMultipleOfFive() {
        $this->beConstructedWith(1);
        $this::fromInteger(20)->shouldHaveType(FizzBuzzNumber::class);
        $this::fromInteger(20)->__toString()->shouldBe("buzz");
    }

    function it_returnsFizzBuzzWhenCreatingFromIntegerWithMultipleOfFifteen() {
        $this->beConstructedWith(1);
        $this::fromInteger(30)->shouldHaveType(FizzBuzzNumber::class);
        $this::fromInteger(30)->__toString()->shouldBe("fizzbuzz");
    }

    function it_returnsAStringFromToString() {
        $this->beConstructedWith(1);
        $this->__toString()->shouldBeString();
    }
}



(I'm writing this part of this article about a week later, and I still can't stand seeing those underscores in my method names!)

But I like what PHPSpec has made me do here: define my FizzBuzzNumber's behaviour:

  • it can be constructed with a value of 1 (a fairly baseline "it works");
  • or a value of "fizz"
  • or "buzz";
  • or "fizzbuzz";
  • or some other integer not matching those rules;
  • but not an integer multiple of three (because there's no such FizzBuzzNumber... that'd be "Fizz");
  • Likewise with a multiple of five
  • (multiple of 15 is implicit from those two).
  • However its fromInteger method can create an object from multiples of 3
  • or 5.
  • And if required to do so, returns a string value of itself (like for output).

That's a reasonable description (ie: specification) of a FizzBuzzNumber, I reckon?

The implementation is pretty simple:

<?php

namespace me\adamcameron\fizzBuzzChallenge;

class FizzBuzzNumber {

    private $value;

    function __construct($value) {
        if (!preg_match('/\d+|fizz|buzz|fizzbuzz/', $value)) {
            throw new \InvalidArgumentException("$value is not a valid FizzBuzzNumber");
        }
        if (is_integer($value) && !($value % 3 && $value % 5)){
            throw new \InvalidArgumentException("$value is not a valid FizzBuzzNumber");
        }

        $this->value = $value;
    }

    static function fromInteger($i) : FizzBuzzNumber {
        $result = "";

        if ($i % 3 == 0) {
            $result .= "fizz";
        }
        if ($i % 5 == 0) {
            $result .= "buzz";
        }
        if (empty($result)) {
            $result = $i;
        }

        return new self($result);
    }

    function __toString() {
        return (string) $this->value;
    }
}

Notes:

  • The constructor does nothing clever. It takes a valid FizzBuzzNumber value and rejects values that aren't valid. EG: 15 is not a valid FizzBuzzNumber: there's no such thing. The integer 15 might exist, but the FizzBuzzNumber equivalent is "fizzbuzz".
  • The fromInteger method now holds all the logic of converting an integer to a FizzBuzzNumber, which is pretty much what we've been asked for: take the integers 1-100 and return their FizzBuzzNumbers.
  • We want to treat FizzBuzzNumbers just like we would an integer etc. So if we output it, we just spit out its string value.
Right, so that's our numbers sorted out. Now we need a collection of them. This is pretty devoid of requirement now given the "fizz buzz" logic is within FizzBuzzNumber, so I made an arbitrary decision to take a slightly more complex route than perhaps I could have. I've decided to use a generator rather than just return the series at once. Why? This is for a job interview right, so it's perhaps good to demonstrate one's understanding of the language extends further than the basics. Also - as a discussion point - it's perhaps worthwhile to observe that this "fizz buzz" exercise is nothing more than a very naive Sieve of Eratosthenes, and that kind of lends itself to be handled via an "on demand" collection, a generator makes sense. But it's mostly one of those "because I can" sort of things. Plus it's dead easy in PHP.

Again, spec first:

<?php

namespace spec\me\adamcameron\fizzBuzzChallenge;

use me\adamcameron\fizzBuzzChallenge\FizzBuzzNumber;
use me\adamcameron\fizzBuzzChallenge\FizzBuzzSequence;
use PhpSpec\ObjectBehavior;
use Prophecy\Argument;

class FizzBuzzSequenceSpec extends ObjectBehavior {

    function its_class_IsInitializable() {
        $this->shouldHaveType(FizzBuzzSequence::class);
    }

    function it_returnsaGeneratorfromGetFizzBuzzSequence() {
        $this->getFizzBuzzSequence()->shouldHaveType(\Generator::class);
    }

    function it_fulfilsTheRequirement(){
        $lengthToTest = 100;
        $this->beConstructedWith($lengthToTest);

        $expected = array_map(function($i){
            return FizzBuzzNumber::fromInteger($i);
        },range(1, $lengthToTest));

        $this->getFizzBuzzSequence()->shouldIterateAs($expected);
    }
}


Notes:

  • This tests that is is a generator.
  • Plus here is the test for the actual requirement! Yup. I don't output it on the screen and ask the examiner to eyeball it manually: I provide a test for them so they can see that the computer has verified my work.

And the actual code is thus:

<?php

namespace me\adamcameron\fizzBuzzChallenge;

class FizzBuzzSequence {

    private $length;

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

    public function getFizzBuzzSequence() {
        $i = 0;
        while (is_null($this->length) || $i < $this->length){
            $i++;
            yield FizzBuzzNumber::fromInteger(($i));
        }
    }

    public function __invoke() {
        return $this->getFizzBuzzSequence();
    }
}

Simple, huh? OK, granted, not as simple as just a foreach loop, but it's not that much more complicated. The only twist is that I've just added an __invoke() method so that the object can be called as a method. Syntactical sugar.

OK, so if yer like me having that test there that proves the thing does the business ain't good enough. I also wrote a "manual test" to demonstrate it:

$seq = new FizzBuzzSequence(100);

foreach ($seq() as $fizzBuzzNumber) {
    echo $fizzBuzzNumber . PHP_EOL;
}

So we still get our simple foreach anyhow. But the implementation is hidden away in suitable classes.

And yeah it works:


C:\src\php\php.local\src\puzzle\fizzbuzz\public>php manualTest.php
1
2
fizz
4
buzz
fizz
7
8
fizz
buzz
11
fizz
13
14
fizzbuzz
16
17
fizz
19
buzz
fizz
22
23
fizz
buzz
26
fizz
28
29
fizzbuzz
31
32
fizz
34
buzz
fizz
37
38
fizz
buzz
41
fizz
43
44
fizzbuzz
46
47
fizz
49
buzz
fizz
52
53
fizz
buzz
56
fizz
58
59
fizzbuzz
61
62
fizz
64
buzz
fizz
67
68
fizz
buzz
71
fizz
73
74
fizzbuzz
76
77
fizz
79
buzz
fizz
82
83
fizz
buzz
86
fizz
88
89
fizzbuzz
91
92
fizz
94
buzz
fizz
97
98
fizz
buzz

C:\src\php\php.local\src\puzzle\fizzbuzz\public>

This is perhaps an overly-belaboured design of something simple, but I still think the design is sound, simple, and well-tested. I think it's an easy amount of work, and demonstrates a few sensible processes that are just as important to a developer job than actual code:

  • be able to read the requirement, understand it, and deliver what's asked for (the hard-coded answer does this). This demonstrates one is paying attention and notices things.
  • But question the requirement in a sensible way: is it just a one use thing? Is "100" cast in stone, or is it a parameter? This demonstrates one can then offer insight into where the client might have overlooked something, or "under-stated" their requirements due to using casual "human speak" when saying things like "I need the first 100 FizzBuzzNumbers". Questioning is always part of one's job.
  • Any work one does should be spec'ed out (but only in a "just in time" fashion, not some horrendous waterfall). Know what yer doing before you set out to do it. It also helps collaboration with other devs, as it summarised things for them.
  • and tested. All our work needs to be tested.
  • Once one has teased-out the requirements from the initial statement, then one can crack on with the implementation, and piss around being "smart". Provided one can explain one's self.

That's the end of "my point" with this article.


As a digression though... obviously with these exercises "seasoned" devs might be wont to show-off how clever they are by making the solution as terse as they can. As if that's a good thing. It's not. But the conversation erred that way on the CFML Slack channel, and I rose to the bait.

Here's my foolishly terse solution to this, in a challenge I set for myself to implement the solution as a single expression in as fewer chars as possible:

[].set(1,100,"").reduce((s,_,i)=>s&(!i%15?"fizzbuzz":!i%3?"fizz":!i%5?"buzz":i),"")

(that only works on Lucee 5, btw. It will not work on ColdFusion)

That's absolutely ludicrous, and if anyone actually submitted that as an answer to the question they should be shot (yes, fine, a harsh penalty for failing a job interview).

But I was quite pleased that CFML can express the solution in one 84-char expression. I thought about trying to implement the same thing in PHP, but it simply can't do this sort of thing. Not being very OO and not doing a very good nod to FP doesn't help. CFML's abilities are def underrated here. In this example of "how to write shit code that'd get you shot".

Righto.

--
Adam