Agile is for losers…

…real men embrace agility.

The development world seems to be full of ‘Agile’ pretenders. If you were to object to my saying that ” there is no such thing as ‘Agile’ ” then I’m sorry to say that there’s a reasonable chance that you too fall into this category. Let me explain…

There is no ‘Agile manifesto’. There is a ‘Manifesto for agile software development’, which lays out some general principles which one might like to follow in order to help them develop software with agility. Of course it seems entirely reasonable to shorten ‘agile software development’ to simply ‘agile’, but herein lies the same problem we see in many other aspects of our day to day lives – words are allowed to be abused and end up losing any sense of meaning. Worse still they end up meaning something entirely different than they did to start with – for example, the word ‘trolling’ as is commonly used in the mainstream media these days has almost no resemblance to its original meaning. We’ve lost a useful word. The ‘agile’ community has suffered an even worse fate as the word ‘agile’, across sweeping parts of the industry, no longer has any relation to the concept of agility. Not only have we lost a useful word – we’ve managed to pervert the philosophy of agile software development as it is understood in many people’s minds.

We’ve reduced agility to mindless process, wrapped up in a pretty new vocabulary. An alert mind will spot the obvious paradox and endeavour to educate themselves to correct their misunderstandings. A keen but less alert mind will suffer the pain of contradiction and endure the inevitable failure (or at least a hefty dose of strife) as long as it takes them to realise that something is wrong; At which point they will deduce either that ‘Agile’ is a fraud, or that they too need to educate themselves so that they might do it properly.

A worrying but very common (in my experience, at least) trend is that of a third kind of mind. This mind persists in having faith that such a prescriptive process (invariably involving moving cards around a board) followed obediently, despite it’s apparently voodoo nature, will yield some kind of magical gain in productivity. These people suffer just like the other 2 kinds I’ve mentioned – but this time they are doomed to forever suffer the pain caused by trying to resolve contradictions, thanks to their stubborn belief in magic.

So, lets see if you’re an agile pretender …

Posted in Uncategorized | Tagged , , | Leave a comment

Story Points, Bugs & asking yourself “To What End?”

Some people like to invent complication. I’ve noticed this a lot when it comes to agile software development among teams that are following the fashion without really understanding what they are doing, or why they are doing it; and it doesn’t help that since seventeen or so guys met up in a ski resort in Utah over ten years ago, a whole cottage industry has sprung up around agile coaching & training – of which I’m certain a significant number lean more towards being snake-oil salesmen selling ‘Agile’ the noun (as Dave Thomas – an original signatory of the agile manifesto – laments here).

I’ve got a few ‘Agile’ items in my backlog to moan about later, but for now I wanted to talk about a particular irritation – story points & bugs. I keep finding myself in discussions with people asking what is the proper way to estimate bugs? Often this apparent difficulty stems from the idea that story points represent business value deliverable to the client – and as such we shouldn’t assign story points to bug fixes because they are mistakes, rather than value to the client and that our velocity should only reflect our ability to deliver value to the client, rather than our ability to clean up after ourselves when we mess up. Here are a few quotes for clarity:

“Our theory is that points, and our resulting velocity, should demonstrate business value to our customers. Customers shouldn’t “pay” for bug fixes. The product should be of a valuable quality.”

“Hone skills for collaborating with the customer. Story points represent what the customer values. Better to hone the skill of writing and estimating stories, as the vehicle for delivering that value, instead of trying to figure out the convoluted ways to write a story about fixing a memory leak.”

“I don’t assign story points to defects since I think story points are a measure of the team’s ability to deliver value in an iteration. Fixing defects, while important and valuable, is NOT delivering value…it’s a drag on the team’s ability to deliver value. The more the quality is built in –> the less defects arise –> the more bandwidth the team has to deliver new stories –> the velocity should be seen to increase. Assigning story points to defects, I think, pollutes this.”

“…fixing defects, while important and valuable, is NOT delivering value…insomuch as value is measured as quality product in the hands of customers. If we’re using story points to measure our effort to make ANY changes to the product, be it new functionality or fixing defects, then have at it and assign story points to defects.

If we’re using it as a measure of a team’s ability to deliver high quality product into the hands of the customer (my preference), then no, don’t assign story points to defect work.”

 

I think that the common theme here that underlies the misunderstanding of story points is the apparent misunderstanding of the term ‘business value’. I think that this is often the case because people have failed to ask themselves the most important question anyone should always ask when developing software – to what end? Perhaps the dark side of the previously mentioned cottage-industry preys on those that are quicker to buy a productised – step by step – solution than they are to wonder: ‘to what end?’ and in the process grok the rationale behind agile development methodologies and principles. Give a man to fish and he’ll eat that evening; Send him on an Certified ScrumMaster course and he’ll progenate a team of jaded and cynical developers who feel no joy in their supposed agility – thanks to the nagging dissonance associated with constantly trying to resolve contradictions which needn’t exist if they only knew ‘to what end?’ they were adopting these methods & techniques, and with that understanding were able to choose and apply them appropriately.

 

Hold on – ‘Bug’ is a pretty ambiguous term…

Yes it is. So to put this post in context, we’re talking about generally well defined defects that have managed to be introduced in a sprint and have found their way outside despite the associated stories being ‘done’. We’re talking about bugs that should have been caught, as opposed to the kind that only manifest themselves when a butterfly flaps its wings a certain way.

Other kinds of bugs include those of similar origin, but which aren’t well understood or defined – these would likely require a spike as it would make no sense to relatively size a poorly understood problem. Although there’s no reason that spike couldn’t spawn point-estimated task if it was worth doing so.

One last scenario worth mentioning is bugs not introduced during a sprint, but the kind that might exist in a backlog to address defects in an existing legacy system. Lets assume that these are all well understood by now so that we don’t need spikes. Should we assign story points to these? No – these probably shouldn’t even be part of the Scrum (i’m assuming Scrum as it’s pretty ubiquitous these days) paradigm – particularly if they are all individual and discrete issues (as they typically are). Why on earth would we think that Scrum or even ‘Agile’ (the noun) is particularly appropriate? To what end would we introduce such a framework?

That’s the subject of another rant I’ll get around to sometime, but to cut a long story short we use agile methods and techniques to achieve a state of agility in the business:

  • Agility to respond to change in a dynamic global market.
  • Agility to fail fast and fail cheap.
  • Agility to minimise time to market in order to maximise return on investment.

 

Those kinds of bug fixes, as discrete individual items, don’t participate in the tight feedback loops that facilitate such agility in the first place. How will Scrum, when fixing these defects, help us to achieve those attributes of agility?

 

To What End?

Story points exist to decouple our estimation process from units of time. It’s as simple as that. Why?

We live our lives in hours and days, and our sprints (if we’re using something like Scrum) are measured in aggregations of those days. Our releases are probably planned in aggregations of those sprints, etc. These are all fixed measurements and their relations, in a quantitative sense, always remain fixed relative to each other (except for leap years perhaps!); The problem that this poses is that it makes it incredibly difficult to estimate accurately (in hours for example), over an extended period of time with all kinds of changing factors which influence our ability to deliver value to the client, just how much value we can deliver in any given timeframe.

If we can decouple our estimations from units of time we can decouple our ability to give useful estimates from the factors that effect the time it takes to deliver – team alterations, changing product complexity, increasing productivity with familiarity, physical environmental factors, or anything else you might imagine.

By sizing stories relative to one another, using points, we can measure our velocity – our ability to deliver value to the client – in a way which is self-correcting without ever having to estimate anything other than how large one story is compared to another.

Different people in different teams can all easily estimate, with reasonable accuracy, the size of one story compared to another – regardless of how long it would actually take each of them to implement those stories compared to their colleagues. With a little effort and discipline we can have discrete teams with different velocities, all normalised according to a common understanding of story point allocation; This grants a product owner otherwise unimaginable flexibility when it comes to planning and allocating stories to multiple teams.

 

So this is the ‘end’ I alluded to when we wondered what story points were used for. So to what end would we conclude that story points shouldn’t be used to estimate bug fixes?

 

Business Value

Simply put, business value is something that the client wants. Something that the client cares about. Something they value.

The client doesn’t care whether you need to spend time upgrading your development machines to the latest OS version, or whether you need to spend 15 minutes every day rebooting your flakey git repo, or whether you need to spend more time testing because quality is slipping (a new screen roughly similar to a previously built screen is worth roughly the same to him whether you spend twice the time or half the time).

Your client DOES care about getting stories delivered, and your client DOES care about emergent bugs being fixed (hence bug fixes do provide business value – otherwise your client wouldn’t care whether you fixed it or not).

With this clearer understanding of business value, is there any end to which you can imagine that not assigning story points to estimate bugs will be beneficial? Lots of people like to assign a spike instead. The trouble with this is that (in the context which I set out earlier) the bug definitely needs to be fixed, and spikes are by definition time boxed efforts (which implies that bug resolution is optional). Secondly, which ever word you use to describe the allocation of resource involves an implicit conversion from time to story points – otherwise how do we know how many points fewer to commit to this sprint? The point of time-boxing spikes is that it makes no sense to try to size them relative to the usual stories; If it does make sense to compare them to the size of stories, then it almost certainly shouldn’t be a spike. The only reason to do so would be to accommodate this faulty ‘bug fixes aren’t business value’ logic.

 

How I would account for Bugs

In the context outlined earlier, I like to keep it as simple as possible unless there is a compelling reason not to. I would size the bug relative to other stories as usual – lets say we think that it will be roughly 2 points. It’s important that we assign these two points because they represent work that needs to be done, which means that there is 2 points worth of other work that won’t fit into the sprint.

We also need to remember that the original story from which this bug leaked (if we can pin it down like that) was estimated with a definition of done which, presumably, implied that the deliverable was bug-free. The fact that we’re now addressing that bug means that the original story isn’t in fact complete, and if we kept our velocity as it is we would go into our next sprint estimating how much work we can ‘nearly finish’, rather than actually finish (especially if this bug leakage is becoming habitual – otherwise in a one off situation this may not be worth the bother). So I’d reduce our velocity by 2 points, for example, too. That means that this sprint we’ll actually plan to deliver 4 points-worth fewer stories, and then going forward our velocity will be 2 points less until such time that we stop leaking bugs and manage to gain some velocity back.

 

Posted in software | Tagged , , | Leave a comment

Java lacks multiple inheritance of implementation because it’s not necessary?

Well… you could argue that Java is unnecessary when we have C. You could argue that C is unnecessary when we have assembly code.  So the question should be – is it useful? Lots of people suggest no – it’s dangerous and we can achieve the same thing through interfaces and composition… Deadly Diamond of Death… Think of the children!

My personal view is that yes, it can be useful. And yes it can be confusing/dangerous. I’d also argue that a spoon can be dangerous in the hands of a moron, yet we don’t abolish the use of spoons, because they have value; And the decision to use them in a dangerous manner is entirely voluntary. I want multiple inheritance of implementation, and I don’t like it when other people say that I should’t have it because they can’t trust themselves not to break stuff.

Here’s one line of reasoning I’d like to address. Someone recently stated in a sweeping and absolute manner:

“Because there is a better way without multiple inheritance.”

So lets look at this example of the observer pattern which I’ve roughly taken from an old Bob Martin document…
We have a Clock which is a self contained class which understands the way in which time is measured and represented. It ticks every second. We’d like to keep it self-contained and with a single responsibility.

We also want to implement an Observer pattern, whereby objects can register themselves as Observers of the clock so that they receive a notification whenever the clock updates it’s internal representation of the time (we don’t want to continuously poll the clock, wasting cpu cycles, when the time will only ever be updated once every second).

Given that Java doesn’t support multiple inheritance of implementations (ignoring some blurred lines introduced by Java 8 just recently), an implementation of the Observer pattern would have to look roughly like this:

public class Clock {

   public void tick() {
      // Update time each tick.
   }
}

interface Observer {
   public void update();
}

interface Subject {
   public void notifyObservers();
   public void registerObserver(Observer observer);
}

class SubjectImpl implements Subject {

   List<Observer> observers = new ArrayList<>();

   @Override
   public void notifyObservers() {
      for(Observer observer : observers) {
         observer.update();
      }
   }

   @Override
   public void registerObserver(Observer observer) {
      this.observers.add(observer);
   }
}

class ObservedClock extends Clock implements Subject {

   private Subject subjectImpl = new SubjectImpl();

   @Override
   public void tick() {
      super.tick();
      notifyObservers();
   }

   @Override
   public void notifyObservers() {
      subjectImpl.notifyObservers();
   }

   @Override
   public void registerObserver(Observer observer) {
      subjectImpl.registerObserver(observer);
   }
}

 

Now compare the above code with the implementation using a hypothetical Java which supported multiple inheritance of implementations:


class MIObservedClock extends Clock, Subject {

   @Override
   public void tick() {
      super.tick();
      notifyObservers();
   }
}

class Clock {

   public void tick() {
      // Update time each tick.
   }
}

interface Observer {
   public void update();
}

class Subject {

   List<Observer> observers = new ArrayList<>();

   public void notifyObservers() {
      for(Observer observer : observers) {
         observer.update();
      }
   }

   public void registerObserver(Observer observer) {
      this.observers.add(observer);
   }
}

 

Now… did writing that code unleash any zombie apocalypse? Which version is cleaner? Which is more elegant? Which is more decoupled? Which is ‘better’ ?

I’d be very interested to hear if anyone can demonstrate a better solution using standard Java.

I’ve heard ‘Uncle’ Bob refer to Java interfaces as ‘a hack’, made for the sake of simplicity – but not for us. For the sake of the JVM implementation. I have no way of knowing whether this is true – the road to hell is paved with good intentions, as they say; perhaps James Gosling and the gang were genuinely trying to look out for us. But what is certain in my mind is that Java is inferior for it’s lack of multiple inheritance of implementation.

Would Java have become such a successful language with multiple inheritance of implementation? Well thats a whole other can of worms.

Posted in coding, java, software, Uncategorized | Tagged , , | Leave a comment

Java is always pass by value

Ok, so this is explained ad infinitum elsewhere on the net. It is also, however, a question (is <some language> pass by reference or value?) asked over and over again by newbies.

I’m not going to explain the differences between the two (or 3!) for newbies right now, but what I’m more interested in talking about is whether it is right to refer to Java as being ‘pass by reference’ or ‘pass by value’ – on the assumption that we all know what the concepts involve.

In the Java community it is understood that Java is ‘pass by value’, because Object method arguments (we’ll ignore primitives) take the form of copies of the values of Object references. I.e. another reference to the same object. A method parameter is never the actual Object reference supplied as an argument.

In the Ruby community (I think) the exact same semantics are considered to be ‘pass by reference’, because what you’re actually passing around is always a reference to the Object argument – never the actual Object itself.

So who’s right?

Now, I seem to be constantly telling people that words exist to describe reality – they don’t define reality. And for this reason I’m happy with both the Java & Ruby conventions, as they both make sense given some context. But… there are 2 reasons why I believe that ‘pass by value’ is more precise and useful, and 1 reason why it’s less useful.

In both of these languages, method arguments are eVALUated first, and the result of that evaluation forms the formal method parameter. I believe that the ‘by reference’/’by value’ language is originally compiler terminology and pre-dates both languages, and in such usage ‘by reference’ would refer to the situation where an expression is not evaluated first, and the actual reference is used. For this reason I believe that the Java camp is using the terminology more precisely.

Secondly, differentiating between reference & value allows us to distinguish between standard Java/Ruby semantics, and the ability to actually pass pure references in languages such as C++ (using the & symbol). Clearly this is an important distinction where such references are mutable for the same reasons that it would be important to note whether your local key-cutter was able only to copy house keys – or whether he was also able to alter a key so that it could open someone else’s house! So again the Java communities choice is more meaningful in that sense.

So here’s the downside of describing Java as ‘pass by value': doing so, obviously, obscures the fact that Java always passes Object arguments as references, and never the actual Object itself. It doesn’t really matter for the most part – but it’s not helpful to new programmers.

So we need a third term! ‘Pass by object sharing’.

Barbara Liskov, who put the L in SOLID, coined the term to avoid these difficulties. ‘Pass by sharing’ describes the semantics without implying anything about the underlying mechanism.

So if someone ever asked be in an interview (I should hope not by now) whether Java was pass by reference or value? I could say ‘yes’. More probably I would say that according to the wider Java community it is ‘pass by value’ – because arguments are evaluated before being used as formal method parameters – but I prefer the term ‘pass by object sharing’.

Posted in coding, java, software | Tagged , , , | Leave a comment

Simple Java DSL with fluent interface

To date, I’ve not put an awful lot of time into considering when it is and isn’t appropriate to introduce a Domain Specific Language into a Java application. It’s something I’ve just started to pay some attention to, having thus far acted on gut feeling that it could be useful in some particular scenario.

Here I’ll present a very small one which I created recently to perform password validation (that is to validate the proposed password meets some minimum strength criteria/standard) in place of an existing static utility method which needed duplicating, with additional requirements, in order to validate a particular flavour of password. I’m taking some time to decide whether it’s actually an appropriate pattern of usage or not.

Here’s the client code making use of it’s fluent interface:

private static final PasswordValidator SECRET_STRENGTH_VALIDATOR = new PasswordValidator()
  .mustContain().atLeast(SECRET_WORD_MIN).and().noMoreThan(SECRET_WORD_MAX).charactersInTotal()
  .mustContain().atLeast(1).numbers()
  .mustContain().atLeast(1).ofTheseCharacters(ALPHA_CHARS)
  .mustContain().atLeast(1).ofTheseCharacters(SPECIAL_CHARACTERS_STRING)
  .mustOnlyContain().anyOfTheseCharacters(ALPHA_CHARS + SPECIAL_CHARACTERS_STRING).or().numbers().andNothingElse()
  .mustBeMixedCase();



Lets for now ignore the fact that this isn’t injected into the dependant client components – there’s only so much refactoring that one can or should attempt in one sitting.

I like this because it reads pretty much like an English sentence (not that ‘English’ or ‘French’, etc, is what the ‘Language’ stands for in DSL, as an old colleague of mine points out here – but in this particular case I think it’s a nice read-ability bonus), rather than a series of methods which must be read through in their entirety, or worse – a big old uber-method which does all of the validation in one hit, before it’s possible to know exactly what the particular validation rules are.

Of course all of that validation code still exists, but now it’s hidden away in validation rules which are constructed according to however the client code makes use of the possible combinations provided by the DSL, and executed when the validate() method is invoked on the validator. In this way many combinations of validation rules can be applied to different validator instances in a self describing way, using minimal extra code to do so.

So lets see where the complexity has been hidden:

public class PasswordValidator {

  private List rules = new ArrayList();

  public boolean validate(String passwordString) {
    char[] password = passwordString == null ? new char[0] : passwordString.toCharArray();
    boolean result = true;
    for(PasswordValidationRule rule : rules) {
      result = result && rule.execute(password);
    }
    return result;
  }

  public MustContainRule mustContain(){
    MustContainRule rule = new MustContainRule(this);
    rules.add(rule);
    return rule;
  }

  public MustOnlyContainRule mustOnlyContain() {
    MustOnlyContainRule rule = new MustOnlyContainRule(this);
    rules.add(rule);
    return rule;
  }

  public PasswordValidator mustBeMixedCase() {
    rules.add(new PasswordValidationRule() {
      @Override
      public boolean execute(char[] password) {
        String passwordString = new String(password);
        String upper = passwordString.toUpperCase(Locale.ENGLISH);
        String lower = passwordString.toLowerCase(Locale.ENGLISH);
        return (passwordString.equals(lower) | passwordString.equals(upper)) == false;
      }
    });
    return this;
  }

}



Ok. So there isn’t anything too complicated here; just a validator with 3 kinds of validation configurable – mixed case, must have something and must ONLY have something. The mixed case requirement is handled here directly as it is a complete requirement in it’s own right. The other two kinds of requirement are meaningless without some further input (what exactly is it that we must have? Must we have a certain number of that thing? etc). You can see that the rules are constructed to a PasswordValidationRule interface (which specifies the execute() method).

Here’s where those extra details are constructed (I won’t show the must ONLY have option – it’s the same but slightly more simple):

public class MustContainRule implements PasswordValidationRule {
  private String chars = null;
  private boolean numbers = false;
  private int from = 0;
  private int to = 0;
  private MustContainRuleDetailAppender detailAppender;

  MustContainRule(PasswordValidator passwordValidator) {
    this.detailAppender = new MustContainRuleDetailAppender(this, passwordValidator);
  }

  public MustContainRuleDetailAppender atLeast(int num) {
    from = num;
    return detailAppender;
  }

  public MustContainRuleDetailAppender noMoreThan(int num) {
    to = num;
    return detailAppender;
  }

  @Override
  public boolean execute(char[] password) {
    boolean isValid =
         validateParticularCharacterRequirements(password)
      && validateOverallLengthRequirements(password)
      && validateNumberRequirements(password);

    return isValid;
  }

  private boolean validateParticularCharacterRequirements(char[] password) {
    if(chars == null) {
      return true;
    }

    int count = 0;
    for (char passLetter : password) {
      count += StringUtils.countMatches(chars, String.valueOf(passLetter));
    }

    return validateCount(count);
  }

  private boolean validateOverallLengthRequirements(char[] password) {
    if(chars != null) {
      return true;
    }
    return  validateCount(password.length);
  }

  private boolean validateNumberRequirements(char[] password) {
    if(numbers == false) {
      return true;
    }
    int count = 0;
    for (char passLetter : password) {
      if(Character.isDigit(passLetter)) {
        count++;
      }
    }
    return validateCount(count);
  }

  private boolean validateCount(int num) {
    boolean pass = from > 0 ? num >= from : true;
    pass = pass && (to > 0 ? num <= to: true);
    return pass;
  }

  public class MustContainRuleDetailAppender {

    private final MustContainRule mustContainRule;
    private final PasswordValidator passwordValidator;

    private MustContainRuleDetailAppender(MustContainRule mustContainRule, PasswordValidator passwordValidator) {
      this.mustContainRule = mustContainRule;
      this.passwordValidator = passwordValidator;
    }

    public MustContainRule and() {
      return mustContainRule;
    }

    public PasswordValidator ofTheseCharacters(String str) {
      mustContainRule.chars = str;
      return passwordValidator;
    }

    public PasswordValidator charactersInTotal() {
      return passwordValidator;
    }

    public PasswordValidator numbers() {
      mustContainRule.numbers = true;
      return passwordValidator;
    }

  }

}

 

And that’s it.
Here we can see that this more complicated rule makes use of a public inner class – this is required in order to limit the client code to make only valid combinations of validation rule detail, such that the client code is only able to construct a single validation rule, in it’s entirety, at a time. This keeps the validation rule construction code simple (and therefore better tested) and the client code is forced to be written in an easy to understand way. This is achieved by controlling which methods are available depending on which object type is returned from each method, and therefore the methods which are then available in the chain; as such being forced to use the api in a specific way is not a bad thing as it actually limits the client code to invoking only a few sensible options at a time. For example, after

  .mustContain().atLeast(SOME_NUMBER)

the client code only has the option of invoking and() in order to add a numerical limit to that same validation rule currently being constructed, ofTheseCharacters() to specify the character set of which SOME_NUMBER must be used, numbers() to specify that SOME_NUMBER of numbers must be present, or finally charactersInTotal() to indicate that the there must be at least SOME_NUMBER of characters total in the password. Each of these methods, apart from and(), returns the original validator object which means that any further validation criteria must exist as another ValidationRule object. Because the MustContain and MustOnlyContain rules are so tightly coupled with their ….DetailAppender classes they have been implemented as inner classes; they’re effectively part of one larger class, but using this technique it’s possible to simulate a kind of context dependant method accessibility (unless the client code decides to deliberately break the mechanism by not chaining their invocations).

DSL_Methods


Why am I concerned about using this pattern rather than simply implementing the rules as private methods in the Utility class, or even as separate rules still, but manually added to a list and executed in sequence? To be honest the more I think about it the better I feel; in this case I feel like I can justify the marginal one off increase in complexity for the reusable simplicity and readability.

The DSL apparatus itself is really pretty simple, and as such it can easily be unit tested with confidence; and with that being the case it feels as though it is actually safer to now implement various validation routines using the DSL’s guiding hand, like this:

private static final PasswordValidator STRENGTH_VALIDATOR_1 = new PasswordValidator()
    .mustContain().atLeast(8).and().noMoreThan(20).charactersInTotal()
    .mustContain().atLeast(2).numbers()
    .mustContain().atLeast(3).ofTheseCharacters(ALPHA_CHARS)
    .mustContain().atLeast(1).ofTheseCharacters(SPECIAL_CHARACTERS)
    .mustOnlyContain().anyOfTheseCharacters(ALPHA_CHARS + SPECIAL_CHARACTERS).or().numbers().andNothingElse()
    .mustBeMixedCase();

private static final PasswordValidator STRENGTH_VALIDATOR_2 = new PasswordValidator()
    .mustContain().atLeast(8).and().noMoreThan(20).charactersInTotal()
    .mustBeMixedCase();

private static final PasswordValidator STRENGTH_VALIDATOR_3 = new PasswordValidator()
    .mustContain().atLeast(10).charactersInTotal()
    .mustContain().noMoreThan(3).numbers()
    .mustOnlyContain().anyOfTheseCharacters(ALPHA_CHARS).or().numbers().andNothingElse();



…than it would be to code them up in the typical, and less structured way. It makes it easier to compare the difference between various validation routines, avoid bugs or assumed behaviour which doesn’t actually exist (which I discovered in the old code during this process, although decent tests should have highlighted that in the first place), and helps to keep that utility class closer to 1 thousand lines than 2 thousand o.O

I’d be interested to hear any criticisms or pointers. Since doing this I’ve been able to cut/paste the code which builds up the validator into emails to send to other non/less-technical people that have needed some information regarding what password-type validation is performed in various places, and it’s worked pretty well. Although it’s a bit of a leap to label this code an internal DSL, it has been easy to share with those less technical people (and is extra convenient for the techies too) because like something such as SQL it has an enforced structure, uses a limited syntax, and is relatively expressive in a self-describing way.

I’m going to go on a hunt now for other scenarios where this kind of mini DSL approach also adds value from an internal perspective, and I’ll add to this if I come up with anything compelling.

Image | Posted on by | Tagged , , | Leave a comment

Boy-scout refactoring

I recently had a puzzling discussion with another  developer regarding refactoring; more specifically the use of private methods – he didn’t like them, favouring giant public methods which presumably require a whole load of inline comments to explain what it’s doing at various (probably deeply nested) stages.

Anyway, I thought I’d post a trivial example of method refactoring as these kinds of discussions tend to pop up occasionally, and this way I don’t have to repeat myself. Moreover the all-seeing eye of Google might benefit a novice to the craft looking for some guidance.

The following before & after snippets are production code (albeit slightly disguised) whereby a user is attempting to set/update a secret passcode which is used elsewhere in a hashing algorithm in order to verify the authenticity of any transmitted messages for that user. Ultimately the routine invokes the static PasswordUtil.validatePassword() (which validates that the new password supplied meets the minimum strength requirements – e.g. special characters, mixed case, etc) and returns the result as well as doing any cleanup required. My task wasn’t to alter this code, but to modify the implementation of PasswordUtil.validatePassword() which this method invokes.

Here’s the original method:

protected boolean validate() {
  boolean ok = true;

  if (JaasUserUtil.hasPermission("Use New Profile Pages")
      && JaasUserUtil.hasPermission("Change redirect SECRET")) {

    EnvironmentPropertiesDataBean data = (EnvironmentPropertiesDataBean)retrieveBean();

    // verify if a newly entered SECRET is secure enough
    if (!data.getDisableRedirectSecret() && data.getRedirectSecret()!=null) {
      ok = data.getRedirectSecret().length() <= 20
          && PasswordUtil.validatePassword(data.getRedirectSecret());
      if (!ok) {
        data.addFeedbackLine(new FeedbackItem(FeedbackItem.Type.WARNING,
        new BundleKey("account_profile.environment.account_secret_invalid",
        new String[]{data.getAccountCode()})));
        // SECRET value entered by the user is invalid -- now, if the SECRET *WAS* empty (disabled), then make sure
        // that it remains that way. Otherwise, the user would be told 'SECRET is set' while that's not actually true!
        String originalValue = AccountCache.get().getAccount(data.getAccountKey()).getRedirectSecret();
        if (originalValue==null) {      // It was disabled;
          data.setDisableaRedirectSecret(true);    // so, make sure it remains disabled.
        }
        data.setResult(FAILURE);
      }
    }
  }
  return ok;
}

Here you can see that the method requires some study in order to determine exactly what it does, and I think genuinely requires the provided inline comments in order to elucidate that behaviour. To start with the very first comment on line 9 would be entirely redundant if only the method had a self-describing name (such as ‘validatePasswordStrength’ rather than plain old ‘validatePassword’). In this particular instance I didn’t change the method name in order to minimise my refactoring footprint, but you will see in the refactored example that the static method on PasswordUtil now has a different name which makes it’s purpose clearer, and in turn the purpose of it’s invoking code is clearer.

You should also see that the latter comments are definitely necessary in this snippet if one wants to get a quick overview of the functionality without having to think about the code. The problem with code comments is that there is no mechanism to force them to stay up to date (i.e. we often find comments in code which say that it does something entirely different that it in actual fact does), and apart from looking untidy a reasonable assumption to make is that the code which they describe is almost certainly more complex than it needs to be (otherwise why write an explanation of what it does?). So comments in code tend to indicate a ‘code smell’ – they hint that something probably isn’t as good as it could be, or typically, much worse.

Below is a refactored version of the same method. It’s not perfect, and it’s not meant to be – it is however significantly better than it was when I found it. The purpose of taking a boy-scout approach is not to make the codebase perfect (perfection costs more money than it saves), but to encourage it’s maintainability.

protected boolean validate() {
  EnvironmentPropertiesDataBean data = (EnvironmentPropertiesDataBean) retrieveBean();
  return validationNotApplicable(data) || validateSecretStrength(data);
}

private boolean validationNotApplicable(EnvironmentPropertiesDataBean data ) {
return (data.getRedirectSecret() == null || !userHasPermissionToChangeSecret() || data.getDisableRedirectSecret());
}

private boolean userHasPermissionToChangeSecret() {
  return JaasUserUtil.hasPermission("Use New Profile Pages") && JaasUserUtil.hasPermission("Change redirect SECRET");
}

private boolean validateSecretStrength(EnvironmentPropertiesDataBean data) {
  boolean result = PasswordUtil.validateSecretStrength(data.getRedirectSecret());
  if(!result) {
    handleInvalidSecret(data);
  }
  return result;
}

private void handleInvalidSecret(EnvironmentPropertiesDataBean data) {
  BundleKey bundleKey =  new BundleKey("account_profile.environment.account_secret_invalid", new String[]{data.getAccountCode()});
  data.addFeedbackLine(new FeedbackItem(FeedbackItem.Type.WARNING, bundleKey));
  resetDisabledRedirectSecretToOriginalState(data);
  data.setResult(FAILURE);
}

private void resetDisabledRedirectSecretToOriginalState(EnvironmentPropertiesDataBean data) {
  String originalValue = AccountCache.get().getAccount(data.getAccountKey()).getRedirectSecret();
  if (originalValue == null) {
    data.setDisableRedirectSecret(true);
  }
}

Ok… Hopefully the ‘after’ version of the snippet should be much easier to read. The idea is that it reads more like plain English – like a story. We’ve minimised having to remember that we’re already nested 3 conditional statements deep – we’ve separated all of the different responsibilities out into single private methods which handle only one or two responsibilities each. We’ve removed the need to assign and later refer to boolean flags. The descriptive method names mean that comments are no longer necessary, and that we should be able to read the code from top to bottom, left to right, and understand the sequence of checks and tasks with the minimum of effort – the new methods are written in the order that they are invoked, and contextual highlighting in modern IDEs make following them even easier.

As a result of this refactoring, which didn’t involve altering any program logic, I am now able to understand clearly the logic contained within this method which means that I can be more certain that I’m doing the correct integration level testing, as well as the unit tests which will run for my altered code which this unmodified logic invokes. In fact during the same piece of work, elsewhere, following this process identified a few bugs, and a swathe of dead code which was otherwise hidden under unnecessary complexity.

As I’ve hopefully demonstrated, proactively refactoring as we interact with existing code not only prevents the code-rot from setting in, but also can help us to do a better job in the moment; it’s not just a good idea – it’s your responsibility as a professional engineer (as I’m sure you’ve heard before).

I’m still dealing, seven years later,  with the aftermath of my house’s previous owner’s DIY electrics. The difference between someone who can do the work, and a professional who can do a proper job really shows in both contexts.

Posted in coding, java, software | Leave a comment

Java 6 IKM test

I had to do a Java 6 IKM test the other day. I thought i’d post the result sheet up because it looks whizzy.

You’ll notice that there are two scores – a percentage and a percentile. I think that the percentile is the important one, as 2 people can get the same percentage accuracy while one answers much harder questions due to the adaptive nature of the test (plus time might play some part in the percentile score too).

Screen Shot 2014-02-04 at 22.27.40

It’s hard. Way harder than the SCJP or whatever Oracle call it these days. I had the advantage of being able to bone-up over the weekend as I was expecting this test to come my way.

It’s an adaptive test which means that the difficulty adjusts on-the-fly depending on how well you are performing. This makes it a better test, but BEWARE… because of this you cannot skip a question and return to it later.

There can be up to four (if I remember correctly) correct answers out of 5-ish options, and the points you get for your answer will depend on how close your chosen answer/s are to the correct answer. This means that, for example, you may score partial credit for selecting a reasonable, even though incorrect answer. In the same way you will score negatively for selecting an entirely nonsense answer. I like this mechanism as it accounts for random guesses, while rewarding partial understanding with partial credit.

There was a time limit of about 2 hours.

Posted in coding, java, software | Leave a comment