Monthly Archives: June 2015

What do you do when your client is wrong?

I blogged last week about how Professionalism means telling the customer when they are wrong. Gonzales commented with a really important question – “What if the client is not just wrong, but stubborn too?”

This has happened to me and to you. You feel that there’s got to be a better end result than compromising your professionalism and delivering software that you know doesn’t help your client as much as it could. You want to take pride in your work.

There is a way to stick to your principles and still keep a positive relationship with your client. It’s all down to how you present your case and how you listen to their case. To make it easy to remember, there are 3 steps based on a technique called “Principled Negotiation” you can follow which really work – and when you pull this trick out people will know it was your professionalism and skilled negotiation that got everyone to a better result.

Suppose your project has already started and there is already financial commitment from both sides. The details are being worked out in an Agile fashion as the project progresses and so part way through your client contact, Jessica, sends you a friendly email that says

“This iteration, could you store passwords in the database as plain text so we can email them out to our users later. Looking forward to seeing it ASAP.”

It’s easy to feel like taking an absolute “no” stance would jeopardise the project – even though you are doing it to prevent your client from causing harm to herself and her users. You can’t risk those passwords going public.

To you, it’s a very clear cut case that you are right. You may even be liable if you agree to do as she asks and later somebody sues.

Positional Bargaining with your clients is a bad plan. You could waste a lot of time and create bad feeling by arguing “we know technology and you’re wrong – it should be like this,” while Jessica says “we know our users and you’re wrong – it should be like this.”

You need a different method. You need Principled Negotiation.

There’s a whole book on the idea, but stick with me, I’ll break it down for you.

1. Get both parties’ interests on the table

What do you want? Most likely your goals are not contradictory. In this example, you want users’ security protected in the case of email interception and in the case of database breach; Jessica wants the process to be easy – or maybe she is thinking ahead to another feature they haven’t mentioned, for example in some cases plain text passwords have been stored in order to allow users to authenticate with a customer services operator by phone.

Once your interests are clearer, you may be able to invent other solutions that satisfy both of you – e.g. there are documented solutions using multiple different types of secrets to enable telephone authentication without risking the users’ primary passwords.

You need to separate the people from the problem. Jessica isn’t stupid. She’s an expert on her business goals. But she might not understand or value your goals. Actively listen to her – this will help encourage her to listen to you too.

If your first response to an issue during development is to consult your contract and figure out what the minimum work you can contractually do is, then you may permanently damage your relationship with her. It should be a last resort, and one that you and she both know was needed because negotiation broke down.

So find the common ground first.

2. Insist on objective criteria for the negotiation

Some of your goals might still actually be in direct opposition. For example, when I interview for a new job, I want the highest possible salary; my employer wants to give me the lowest possible salary. If you can’t solve the issue just by talking through what else each party can bring to the table, then the next strategy is to appeal to an independent source.

This doesn’t necessarily mean you need to bring in a consultant. In the case of you hiring me as a software developer, we could agree that we will start from using market data on average salaries for developers with similar experience. And then we just need to work out the value of your company’s unique benefits and my specialist knowledge, and adjust accordingly.

In the case of the plain text password feature, we could all agree (preferably before the outset of the project) that if we reach a disagreement like this, that we will refer to an unbiased external authority. In this particular case, we could agree that external authority to be Troy Hunt’s blog.

You could also introduce your own custom external authority before the start of the project in the form of a code of ethics.

Your ethical policy might include something like “We believe in protecting users’ private details.”

UPDATE: Since the updates to data protection legislation in the UK and Europe it’s becoming clearer and clearer that agreeing in this example would make you legally exposed – so it’s easy to say “we won’t break the law”. The argument I was making was supposed to be about a slightly greyer area – what do you do if your client asks you to be unethical but not illegal. I’ll expand on this topic and update soon.

Your ethics are unbiased; they apply equally to all projects, and there is nothing personal – and therefore confrontational – about you appealing to your company’s ethical policy during a dispute.

3. Have a fall back position

It is most often in the interest of both parties to reach agreement. But sometimes it really isn’t. By having thought through the fall back plan, you can make it easier to stick to your principles. If your client attempts to strong-arm you into compliance with threats, you can make your fall back position known.

Having a code of pre-disclosed ethics or a contract with get-out clauses helps with this. For example, a friend of mine working in the veterinary profession had her clients sign an agreement before their animals undergo surgery – if there is an unforeseen complication and Fluffy’s owner cannot be contacted, then they contractually agree in advance to a fall back plan – that the vet will use their medical expertise to take whatever action is in Fluffy’s own best interests. The principled appeal here is to the idea of what the animal would want, and this is something that other independent vets can agree on.

You may have several fall back plans

  • kill the project (extreme and costly, but always an option)
  • do not develop (and do not charge for) the authenticated features of the project
    – in some projects this may be viable.
  • develop exactly what was in the initial contract (assuming the ethical violation wasn’t something you already agreed to)

Given these plans, you can now more confidently say to Jessica that if you cannot reach an agreement then you may not do the work.

In light of your fall back plan, alternative implementations may be more appealing to her – such as just going with your recommendation, or perhaps agreeing to use a 3rd party provider e.g. OpenID.

It isn’t always that hard

So what about less clear cut cases – maybe you want to encourage a client to use a different UI than then one they propose because you believe it will be more effective?

These negotiations should actually be easier, because less is at stake. Your fall back position may be “do what the client says”.

In terms of Principled Negotiation, the objective criteria you bring to the table might be in the form of referring to past projects or published articles on design. You and your client share the same goal – high user engagement. Perhaps you could invent a solution that works for both of you; for example using analytics and A-B testing to determine which UI users prefer.
Clients may not even be aware of the usage data you could capture – this could be a real value-added for them, helping them know that the application is the best it could possibly be.

I strongly believe that digital agencies regularly sell their own skills short by blindly following their clients’ plans rather than bringing all their expertise to projects and acting as technology experts for hire.

If you’ll indulge me while I make a sweeping generalisation, ALL of the conflicts I’ve seen around features that the client or developers strongly disagree with have stemmed from a lack of communication. All agile methods advocate having the customer as part of the team because constant contact massively improves shared understanding of their business goals and of your technological expertise.

I hear many developers say “my clients are stupid.”

I’ve heard many clients say “my software developers are incompetent.”

Did you ever take the time to listen to each other?

For more about Principled Negotiation, you should buy Getting to Yes by Roger Fisher and William Ury. If you read this book you’ll never think about negotiation the same way again.

Who is the expert?

…in what software the customer wants?
…in how to deliver that to the customer?

Hint: The answer to one of these questions is not “the customer”.

I’ve been given feature requests that include such things as sending passwords by email.

There are 2 major types of response a developer might have to this:

Client: I want the system to email my my password when I forget it.  Yes-Man-Dev: Sure thing boss (compromised security? you asked for it!).  The Professional: That's not great for security - let's talk alternatives.

I would love to say that it’s been a long time since I’ve seen the yes man path taken by a developer, but I guess this is a work in progress. I witnessed this conversation about a project that hadn’t been started yet – but at least one person wanted to implement the inferior solution documented in the agreement; rather than contact the client to get the agreement updated – which would have been in everyone’s best interest and wouldn’t affect the cost.

This sort of thing is far more likely to happen when developers do not have easy and regular contact with stakeholders. If you’re in an organisation that has a long history of waterfall – or chaos – and you’re trying to become Agile, this is one major hurdle to overcome. Sometimes developers cover their asses with a feature list because they’ve been taught that covering asses is what the company does.

But I thought we were here to build great software!

There is a bigger question here. What is your job role?

Are you someone who codes exactly what they are told to code? Are the requirements given to you on a stone tablet from a mountain?

Well, great. If the client never needs technical insight to choose the right solution, if there is nothing you could do better than the author of your specifications, and if the requirements are documented completely, correctly, and never change, then good luck to you.

But that’s not the reality we tend to live in.

When the client is simply starting from the wrong place – “I want to build an app like facebook, with slightly different features, oh and people should pay me to use it.” – Do you advise them that the project is a bust? Or do you charge them for it?

What service would you be proud to say you give?

“I code what my clients tell me to code, after all they’re the experts.”

OR

“I use my deep technical knowledge to help my clients figure out how to achieve their business goals. I can even help them spot opportunities and I help them avoid expensive mistakes.”

Trust me, at the kind of money you are charging your clients, they don’t want you to shoot them in the foot just because they told you to.

You are the expert. Now act like one.

The xUnit team removed my favourite feature… and made me a convert.

So I was helping one of the senior devs here with a code review – let’s call him Pete. Because his name is Pete, so that’ll make things easier.
Anyway Pete asked me for a second opinion on a couple of tests checking for the existence and non-existence of exceptions.

This seemingly ultra-specific topic actually revolved around a few super important general principles – like readability; semantics; and Arrange, Act, Assert.
Its generally accepted that if you ignore these principles your test code quickly turns claggy and unmaintainable.

I was writing up our discussion when I discovered that the xUnit team had removed support for my-favorite-assertion-you-never-heard-of. I’m annoyed of course… but not because they removed it. I’ve been advocating NUnit at my work for some time; we’ve been converting MSTest projects over to it because it’s more expressive.
All of that hard work may need to be redone – because those clever bastards at xUnit had such a damn good reason for doing it they’ve only gone and made me a convert.

I’ll get around to explaining that, but it doesn’t make sense without comparing the tools each framework provides, so bear with me.

Do you know how to test for exceptions in an elegant and expressive way?
Let’s look at some code and see if your implementation comes up.

[TestFixture]
public class When_validating_an_email
{
    [Test]
    public void with_an_invalid_email_it_should_throw()
    {
        var sut = new EmailValidator();
        
        try
        {
            sut.ValidateEmail("foo.bar.com");
            Assert.Fail("it should throw");
        }
        catch (EmailInvalidException e)
        {
        }
    }
}

The test is easy to understand and the naming while not my style is clear enough… but is everything about the test optimal or do we have choices here?

Of course we do.

Our SUT is a validator that is supposed to throw an exception when it is given an invalid email.

If the SUT throws the wrong type of exception, it will not be caught and will bubble to the test runner which will fail the test.
If the SUT fails to throw an exception, execution will continue to the “Assert.Fail” line and fail there.
And finally, if the SUT behaves as expected, then the “Assert.Fail” will be skipped, and we’ll end up swallowing the exception in the catch block.

Actually that’s pretty complicated when you think about it. That’s three very plausible execution paths, and our happy case involves skipping code (the fail statement).

Let’s jump straight to the next option, the most common way I’ve seen this solved.

    [Test, ExpectedException(typeof(EmailInvalidException))]
    public void with_an_invalid_email_it_should_throw()
    {
        var sut = new EmailValidator();
        sut.ValidateEmail("foo.bar.com");
    }

Well we’ve saved a lot of lines of code.

This implementation is courtesy of MsTest and NUnit’s “ExpectedException” attribute.

Unfortunately, most real world test implementations will be a little longer than this test, and the one thing this test doesn’t do is care _which line_ threw the exception… So for example if the constructor throws (which would lead to a very confusing bug) the test actually passes.

A less important, but still valid concern is that the assertion for the test has floated up and away from the test body to exactly where we don’t expect it to be. I like to understand tests within just a few seconds; this layout gives me eye strain. Actually the xUnit team is quite nicely opinionated on this point, and have explicitly chosen not to implement ExpectedException – (but this was never my favorite-assertion-you-never-heard-of – you’ll have to keep reading for that.)

We can do better.

    [Test]
    public void with_an_invalid_email_it_should_throw()
    {
        var sut = new EmailValidator();
        Assert.Throws<EmailInvalidException>(
            () => sut.ValidateEmail("foo.bar.com") );
    }

We have localised the exception to the method we are testing, we have the exact type of exception constrained, and our assertion is sitting at the end of the test where we expect it to be.

I’m not the only one who prefers it. Programmers frustrated with MsTest’s lack of support for this feature have implemented it themselves.

This method is supported by NUnit and xUnit.

As a final note on “Assert.Throws”, take a look at the first test’s ugly brother.

    [Test]
    public void 
with_an_invalid_email_it_should_throw_an_exception_containing_the_email()
    {
        string result = null;
        
        var sut = new EmailValidator();
        
        try
        {
            sut.ValidateEmail("foo.bar.com");
        }
        catch (EmailInvalidException e)
        {
            result = e.Email;
        }
        
        Assert.AreEqual("foo.bar.com", result);
    }

Yuck. {Code blocks} in a test method are a smell that something has gone wrong.

How about this.

    [Test]
    public void 
with_an_invalid_email_it_should_throw_an_exception_containing_the_email()
    {
        var sut = new EmailValidator();
        
        var e = Assert.Throws<EmailInvalidException>(
            () => sut.ValidateEmail("foo.bar.com") );
            
        Assert.AreEqual("foo.bar.com", e.Email);
    }

This is how I currently code this scenario.
xUnit offers an alternative, but we’ll get back to that when we’ve looked at how we can test for no exception.

Every sut that you test throws on invalid input must have a corresponding test for valid input, right?

Let’s see some ways to do that, starting with the one Pete showed me.

    [Test]
    public void with_an_valid_email_it_should_succeed()
    {
        var sut = new EmailValidator();
        
        try
        {
            sut.ValidateEmail("foo@bar.com");
        }
        catch (EmailInvalidException e)
        {
            Assert.Fail("it should not throw");
        }
    }

This is quite interesting, because the author has done something strictly unnecessary in the noble pursuit of having an expressive test.

Compare with this:

    [Test]
    public void with_an_valid_email_it_should_succeed()
    {
        var sut = new EmailValidator();
        sut.ValidateEmail("foo@bar.com");
    }

These tests pass and fail identically… but the first test has code documenting the idea that we anticipate that an exception could happen and we don’t want it to. This idea of documenting that an exception could happen is a good one, but not done very succinctly in the first example and not done at all in the second example.

In fact, if you read the second test, would you even know that the pass condition was “no exception thrown?” There is no assertion in the test, so it looks like the author forgot to write one – perhaps they were interrupted. One could argue the test isn’t doing anything and has no value, and remove the test – accidentally removing an important test case in some code that uses exceptions.

Minor improvement:

    [Test]
    public void with_an_valid_email_it_should_succeed()
    {
        var sut = new EmailValidator();
        sut.ValidateEmail("foo@bar.com");
        Assert.Pass("No exception thrown.");
    }

Now we can tell that we are testing for an exception. Assert.Pass and Assert.Fail are important to know about – but unless you have exceptions with try-catch (which can cause code to be skipped) or control statements in your test you shouldn’t need them for a finished test. I often use Assert.Fail to stub out a failing test just before I am forced to context switch, to remind me what I had to do next.

Let’s get rid of that Assert.Pass. Again NUnit offers an assertion that takes an Action.

    [Test]
    public void with_an_valid_email_it_should_succeed()
    {
        var sut = new EmailValidator();
        Assert.DoesNotThrow(
            () => sut.ValidateEmail("foo@bar.com"));
    }

It’s pretty readable right? This is my favorite-assertion-you-never-heard-of! Like Assert.Throws, it localises the thrown exception to one line – now future readers of the test know the intent and have the idea that this method may throw.

Supposing an exception is raised outside of this statement? Well the NUnit will give different diagnostic feedback about an unhandled exception, rather than about a failed assertion.

This finishes the story for NUnit… So far we’ve seen NUnit consistently provide more expressive options for exception testing than MSTest; and xUnit has seemed great too… but as I already hinted, they removed my favourite assertion.

Assert.DoesNotThrow has been deprecated by the xUnit team! It’s not making the cut for version 2.0!

Surely they can’t be advocating the Assert.Pass version?

I was googling for DoesNotThrow to research opinions and usage when I discovered the offending feature request on github.

Remove Assert.DoesNotThrow #188. BradWilson: So. Incredibly. Not. Needed.
Source

BradWilson: Every single line of code is an implicit

I hope I’ve explained why I disagree with that. Localisation of the exception, readability, and having an assertion in each test are more than reason enough.

Mark Seemann was the first to put the case for the defence. I’m glad someone voiced my views.

Ploeh: I, for one, liked the explicitness of being able to state that this particular line of code shouldn't throw, and that's the test.

If that was it, I would have decided it was a clear cut case – they were just wrong.

But there’s an idealogical issue at stake here. See, the test has a slightly twisted structure. We expect

  • Arrange
  • Act
  • Assert

And what we have instead is

  • Arrange
  • Assert(Act)

Note that Assert.Throws has the same problem. This seems to me to be a minor issue… if the test remains readable. But it is interesting that the xUnit team have taken such a strong opinionated stance.

Adam Ralph made a stronger argument for an alternative.

Personally, I believe neither Assert.Throws<T> nor Assert.DoesNotThrow<T> should exist.  (...you can use Record.Exception instead and preserve the structure of Arrange, Act, Assert)

I would never have thought of writing a helper like “Record.Exception” because it violates a different guideline I hold close to heart – “don’t return null” (because you force the caller to check for it). But the means justify the end in this case – which is to preserve the integrity of Arrange Act Assert.

The discussion on the xUnit project board is inspirational – because people passionate about testing are discussing and discovering best practices and building a framework that is designed around those ideas.

In the end your choice will be a matter of taste. But please remember you have more concise and expressive options than try-catching the exceptions yourself.