Is this a code smell? Writing code to support testing

gistfile1.as

The following two methods use the currentDateTime() method to get the current time,
rather than calling new Date() directly. the only reason I did this was so that I 
could override currentDateTime() in my test case class in order to pass in an 
existing Date object to ensure that the time would not have changed between the 
method being called and the assert made.

Is this bad practice? I can't decide.

 /**
  * Callback method used by _macroSubstitutions to provide the current date in DD_MMM_YYYY format
  */
 protected function generateDate():String
 {
    var date:Date = currentDateTime;
    var formatter:DateFormatter = new DateFormatter();
    formatter.formatString = "DD_MMM_YYYY";
    return formatter.format(date);
 }
		
 /**
  * Callback method used by _macroSubstitutions to provide the current time in HH_MM format
  */
 protected function generateTime():String
 {
   var date:Date = currentDateTime;
   var formatter:DateFormatter = new DateFormatter();
   formatter.formatString = "HH_NN";
   return formatter.format(date);
 }
		
 protected function get currentDateTime():Date
 {
   return new Date();
 }

David,

you said: "TDD fans: is designing code to simply facilitate easy testing a bad thing?"

It depends. See Michael Feathers' talk called 'The Deep Synergy Between Good Design and Testability' http://vimeo.com/15007792 . In it, he says (my pedestrian transcript of some excerpts follows):

KEY THESIS: solving design problems solves testing problems
BUT: making code testable does not necessarily make the design better
so there is this conflict between testability and good design
it does have a direction to it: making design better tends to make things more testable, but there are ways of making the thing more testable that aren't necessarily adherent to good design principles (e.g. the SOLID principles)

You may also find this post useful: http://michaelfeathers.typepad.com/michael_feathers_blog/2007/09/the-deep-synerg.html

Hi David,

the standard technique for doing this is to have a Clock service that is passed in as a collaborator. For instance

interface Clock { Date now(); }
class RealClock { 
    public Date now() { return new Date() }
}
class FakeClock {
    private Date toReturn;
    public FakeClock(Date toReturn) { this.toReturn = toReturn; }
    public Date now() { return toReturn; }
}
class MacroSubstitutions {
    // constructor that takes a Clock service, used by the tests
    public MacroSubstitutions(Clock clock) { this.clock = clock; }
    // default constructor, used in production code
    public MacroSubstitutions() { this(new RealClock()); }
    // ...
}

Extract-and-override is almost always a step towards introducing a new Collaborator. See "Replace Inheritance with Delegation" in Fowler's Refactoring. Introducing the Collaborator inverts the dependency, increasing context independence by pushing dependency on the runtime environment up a level of the call stack. That all sounds good to me.