Yesterday I had the chance to write some code to determine if I needed to schedule an action for later processing, or do the action immediately. I am working on a system that has a fairly well laid out component architecture, with a fairly good Business model. However, it is almost all “Controllers and Data Structures” at this point. Given my previous experiences with how people would write this type of code, it would invariably end up like this:
-
public void someComponentMethod
(Long rootModelID
) {
-
RootModelObject root = DAOFactory.getRootModelDAO().getRootModel(rootModelID);
-
-
// many lines of code later …
-
-
if ( root.getExtraData() != null &&
-
root.getExtraData().getSchedule() != null ) {
-
Schedule s = root.getExtraData().getSchedule();
-
if ( s.
getScheduleDate().
after(new Date()) ||
-
s.getCronExpression() != null ) {
-
root.setStatus(SCHEDULED);
-
DAOFactory.getRootModelDAO().save(root);
-
// schedule for later code
-
}
-
} else {
-
// deliver immediately code
-
}
-
// many lines of code later …
-
}
There are a number of issues in the above code:
- The interface to this method is abhorrent. I will discuss this in another post.
- The component knows WAY too much about the specifics for processing.
- Hand in hand with the above, it knows way too much about the Business Model.
This is a classic case of “Asking, not Telling”, the antithesis to the awesome article Tell, Don’t Ask, by Dave Thomas and Andy Hunt. GO NOW and read it if you haven’t.
Here is the code I ‘ve written so far:
-
public void someComponentMethod(RootModelObject root) {
-
// small amount of other code
-
-
if ( root.shouldBeScheduled() ) {
-
root.setStatus(SCHEDULED);
-
DAOFactory.getRootModelDAO().save(root);
-
scheduleComponent.schedule(root);
-
}
-
else {
-
root.setStatus(PROCESSING);
-
DAOFactory.getRootModelDAO().save(root);
-
nextComponent.process(root);
-
}
-
}
There are few things here that are better: First, I pass in the object I want to work on… this is SO much better than having to go get the Object from some back-end store by ID. I will definitely be talking about this more in the near future. Second, I broke out the beautiful comments from the code above into separate methods, on the components that they represent. This can only make things easier in that code, which most assuredly would have otherwise been in the if block as the comments suggest. And finally the piece that got me started on this post, I moved all of the logic revolving around the Schedule behind the walls of the RootModelObject. But, I did not stop there. Since from the perspective of the RootModelObject, it is not it’s responsibility to determine if it should be scheduled, I created a shouldBeScheduled method on the ExtraData object, which then called a shouldBeScheduled() method on the Schedule object:
-
class RootModelObject {
-
…
-
public boolean shouldBeScheduled() {
-
return getExtraData().shouldBeScheduled();
-
}
-
}
-
-
class ExtraData {
-
…
-
public boolean shouldBeScheduled() {
-
return getSchedule().shouldBeScheduled();
-
}
-
}
-
-
class Schedule {
-
…
-
public boolean shouldBeScheduled() {
-
return ( getScheduleDate
().
after(new Date()) ||
-
getCronExpression() != null );
-
}
-
}
This “string of pearls” makes it much easier to keep the code where it needs to be, while providing convenience to the component developer.
So, the code is better, but part of me says that the code really should look like this:
-
public void someHigherLevelComponentMethod(RootModelObject root) {
-
…
-
root.schedule();
-
}
Since technically, when you say “process immediately” you are effectively saying “schedule for processing now”. What do you think?
by Joshua Kerievsky. This one has been on my wishlist for a long time. I have skimmed the first few chapters and it looks great.
by Alan Watts. I have had this book for quite some time now. Before this time, every time I tried to read it I would get bogged down in the sometime terse language. This time I got through that and now am throuroughly enjoying it, although it still can be difficult at times with the Chinese and Indian names for things.