2009.01.20
Posted in Programming, Ruby at 6:59 pm by The Rontologist
A common task to do in programming is to iterate through a list and test each element in a collection. In most languages this is done exactly how you would think.
strings = ["param1", "param2", "param3"]
passed_all = true;
strings.each {|s|
passed_all = false unless s.length == 6
}
puts "Matched all" if passed_all;
However Ruby is kind enough to give us the any? method on its collections. This allows for doing the same test in more more straightforward manner.
strings = ["param1", "param2", "param3"]
if (strings.all? { |s| s.length == 6 })
puts "Matched all"
end
It also gives us an any? method too if you need to do an action if, you guess it, any item in the collection passes a test.
Permalink
Trackback
2009.01.07
Posted in General Software at 6:35 pm by The Rontologist
I read an interesting article today titled Revenge of the Nerds. It is a good essay about why PHB’s choose languages without knowing about them, and how languages are getting closer to LISP over time. It is a little long but it does have several good points if your into that sort of thing.
Check it out here.
Permalink
Trackback
2008.12.25
Posted in Misc at 12:01 am by The Rontologist
Happy Holidays everyone! I hope you all are having a great time!
And to those who are into the FSM — I gotta make me one of these next year.
RAmen.
Permalink
Trackback
2008.12.02
Posted in Java, Programming at 5:15 pm by The Rontologist
Returning multiple values from a single function is not supported in most programming languages. To get around this limitation some developers have wrapped their return values in an array. This is a bad practice that should not be followed as it leads to fragile and unreadable code.
For example, lets make a function that needs to return a user object and her favourite movie. This can be done by wrapping both pieces of data in an object array like this:
public Object[] getUserInfo(String username) {
/* code to look up a user */
return new Object[] {user, favouriteMovie};
}
This is nice, compact, and easy to implement. Unfortunately it is a bit unwieldy trying to use it:
Object[] info = getUserInfo("bob");
String name = (String) info[0];
Movie favouriteMovie = (Movie) info[1];
There is a fair amount of casting going on, and it does add several extra variables. But it does work, and because of time considerations we decide to leave it be until we have time to go back to it (and since this is software that will never happen).
A week down the road the code gets selected for a random code review. The reviewers are feeling the time pressure as well and decide that the only thing that needs changing are the index literals. It would be much better if they were constants. This is a change that can be done quickly with little risk of making a mistake.
public static final int USER_INFO_USER_IDX = 0;
public static final int USER_INFO_FAV_MOVIE_IDX = 1;
public Object[] getUserInfo(String username) {
/* code to look up a user */
return new Object[] {user, favouriteMovie};
}
// usage
Object[] info = getUserInfo(”bob”);
String name = (String) info[USER_INFO_USER_IDX];
Movie favouriteMovie = (Movie) info[USER_INFO_FAV_MOVIE_IDX];
The constants are in place and everyone agrees that it is an improvement.
Another week down the road a new requirement comes down the pipe requesting that the software also tracks the users favourite snack. It is decided that the getUserInfo function should be expanded to return this information as well.
public static final int USER_INFO_USER_IDX = 0;
public static final int USER_INFO_FAV_MOVIE_IDX = 1;
public static final int USER_INFO_FAV_SNACK_IDX = 2;
public Object[] getUserInfo(String username) {
/* code to look up a user */
return new Object[] {user, favouriteMovie, favouriteSnack};
}
// usage
Object[] info = getUserInfo(”bob”);
String name = (String) info[USER_INFO_USER_IDX];
Movie favouriteMovie = (Movie) info[USER_INFO_FAV_MOVIE_IDX];
Snack favouriteSnack = (Snack) info[USER_INFO_FAV_SNACK_IDX];
As you can see it is getting uglier with each iteration. The focus of our business changes and we no longer care about movies, but they are so ingrained in the system that we will need to take an iterative approach in removing them. The first set is to remove it from the getUserInfo function. Doing it for the function is easy.
public static final int USER_INFO_USER_IDX = 0;
public static final int USER_INFO_FAV_MOVIE_IDX = 1;
public static final int USER_INFO_FAV_SNACK_IDX = 1;
public Object[] getUserInfo(String username) {
/* code to look up a user */
return new Object[] {user, favouriteMovie, favouriteSnack};
}
// usage
Object[] info = getUserInfo(”bob”);
String name = (String) info[USER_INFO_USER_IDX];
Movie favouriteMovie = (Movie) info[USER_INFO_FAV_MOVIE_IDX];
Snack favouriteSnack = (Snack) info[USER_INFO_FAV_SNACK_IDX];
When we try to compile this we will get a warning from every class that used the USER_INFO_FAV_MOVIE_IDX symbol. Once those are fixed up it should be fine, but unfortunately it is not.
When the array is returned there is no way to enforce that the constants are used while accessing it. Any usage of it where developers used literal numbers to access the elements will still compile, and cause runtime errors since info[1] is now returning a Snack instead of a Movie. This can be the source of more bugs than you can shake a stick at, and is precisely the reason why you should never return an array like this.
A better approach would be to create a small object as a container and return that. It is a little more overhead up front but it does pay off during the maintenance cycle. Assume we had implemented getUserInfo loike this prior to the request to remove the favourite movie.
public static class UserInfo {
private String user;
private Movie favouriteMovie;
private Snack favouriteSnack;
/* getters and setters */
}
public UserInfo getUserInfo(String username) {
/* code to look up a user */
return new UserInfo(user, favouriteMovie, favouriteSnack);
}
Not only will the usage of this be nicer (since the casting and temporary variables are no longer needed) but it’ll be easier to maintain to. If we remove the movie property from our object the compiler will let us know every usage of it - not just the ones that people accessed with optional constants.
Arrays as a multiple return mechanism is a bad idea and a horrible code smell.
Permalink
Trackback
2008.11.04
Posted in General Software, Programming at 5:30 pm by The Rontologist
Code coverage reports are neat. At a glimpse you can determine what pieces of code have been executed in a batch of unit tests, and get a good guesstimate on how well your unit testing efforts are going. Unfortunately there is a tendency to jump to the conclusion that covered code is tested code. If only it was that simple!
Assume we have a simple method:
function calculateTotal(amount, includeTaxes) {
if (amount < 0) {
raiseError();
}
var total = amount;
if (includeTaxes) {
total *= 1.05;
}
return total;
}
This can be tested fully with a few simple asserts
assertEquals(100, calculateTotal(100, false));
assertEquals(105, calculateTotal(100, true));
assertEquals(0, calculateTotal(0, false));
assertEquals(0, calculateTotal(0, true));
assertError(calculateTotal(-5. true));
assertError(calculateTotal(-5. false));
Running these tests will provide a coverage report that indicates we have 100% code coverage. That is good, since any fully tested function will have 100% code coverage, and our tool would be broken if it didn’t say that. However, what happens if we need to add a small bit of functionality to this function? Maybe we want to include the option to include a default tip. Our function could be changed like this:
function calculateTotal(amount, includeTaxes, includeTip) {
if (amount < 0) {
raiseError();
}
var total = amount;
if (includeTip) {
total *= 1.15;
}
if (includeTaxes) {
total *= 1.05;
}
return total;
}
Since we have added to the function we need to modify our tests or the compiler will ball at us. There also needs to be additional testing included to cover the new functionality. Ideally we would have done this TDD, but that’s a topic for another day.
//existing tests modified for third param
assertEquals(100, calculateTotal(100, false, false));
assertEquals(105, calculateTotal(100, true, false));
assertEquals(0, calculateTotal(0, false, false));
assertEquals(0, calculateTotal(0, true, false));
assertError(calculateTotal(-5. true, false));
assertError(calculateTotal(-5. false, false));
//new test
assertError(115, calculateTotal(100, false, true));
If we run our tests again we will find that we have 100% code coverage again. It is tempting to assume that it is 100% tested, but jumping to the conclusion is wrong since we did not test how the function behaves if we are calculating both the tip and the taxes. Never assume that you have 100% test coverage if you have 100% code coverage.
Does this mean that the code coverage report is useless? I don’t think so. While you cannot determine if a covered line is tested, you can determine that an uncovered line is not. This may give you a clue on the best place to start your unit testing effort.
Permalink
Trackback
2008.10.21
Posted in Bad Code, Java, Programming at 5:15 pm by The Rontologist
Sometimes you just have to ask why. Why would you ever produce this monstrosity?
void processMovies(List movieList, User user) {
if (user.hasFavouriteMovies()) {
Movie movie;
while (movieList.hasNext()) {
movie = movieList.next();
switch (movie.getRating()) {
case LOVED_IT:
if (movie.hasExplosions()) {
doLovedItWithExplosions(movie);
} else {
doLovedItWithoutExplosions(movie);
}
break;
case HATED_IT:
if (movie.hasExplosions()) {
doHatedItWithExplosions(movie);
} else {
doHatedItWithoutExplosions(movie);
}
break;
default;
if (movie.hasExplosions()) {
doIndifferentItWithExplosions(movie);
} else {
doIndifferentItWithoutExplosions(movie);
}
}
}
}
}
There is no good reason to have an if inside a switch. If you have to do a further conditional in the switch than either break it out into a function or get rid of the switch. It makes it unreadable, especially if it is nested inside two other layers of nesting as in the example above. That is too much to handle.
We can refactor this in multiple ways. I prefer the cases of my switch statement to be small so I will break out the logic into separate functions and thus reducing the cyclic complexity of that code. You could also translate the switch statement into an if/else if/else structure as well, but I do not find that to be as elegant.
void doLovedIt(Movie movie) {
if (movie.hasExplosions()) {
doLovedItWithExplosions(movie);
} else {
doLovedItWithoutExplosions(movie);
}
}
/* doHatedIt, doInfferent */
void processMovies(List movieList, User user) {
if (user.hasFavouriteMovies()) {
Movie movie;
while (movieList.hasNext()) {
movie = movieList.next();
switch (movie.getRating()) {
case LOVED_IT:
doLovedIt(movie);
break;
case HATED_IT:
doHatedIt(movie);
break;
break;
default;
doIndifferentIt(movie);
break;
}
}
}
}
I find that to be much more readable, and it is easier to test. The new functions can be unit tested easily since they are isolated, and the processMovies function can be tested easier as well because it no longer have all the extra ifs to execute.
Permalink
Trackback
2008.09.24
Posted in General Software at 5:08 pm by The Rontologist
A friend on mine pointed me in the direction of Stack Overflow the other day. It is a great resource for developers, and just a little bit addictive. Go check it out if your a developer and haven’t already.
Permalink
Trackback
2008.09.08
Posted in Programming at 5:15 pm by The Rontologist
Conditionals when used correctly can make verbose code concise and readable. However, there are enough examples of them being used incorrectly that can scare most people away from them completely.
$year % 400 ? $year % 100 ? $year % 4 ? false : true : false : true
That is one of the most unreadable conditionals I have ever seen. It returns true of false depending on whether or not $year is a leap year. However this is a bit of an extreme example. Just because something can be used badly doesn’t mean we cannot use it well. If that was the case then most Hollywood actors would be out of work.
The following code sample does a very simple thing. It calls a sort method with a sort direction depending on a flag.
if (flag) {
sort(SORT_ASC);
} else {
sort(SORT_DESC);
}
This is quite verbose, but it does not use conditionals. It can be modified to be even more readable with the use of a simple conditional:
sort(flag ? SORT_ASC : SORT_DESC);
The benefit of doing it this way is that the logic of the sort direction is contained in the function call. There is no need to look at its surrounding lines to figure out what is going in. This means less work, and a win from my point of view.
Permalink
Trackback
2008.09.05
Posted in Java, Javascript, Programming at 5:00 pm by The Rontologist
Sometimes when you are trying to create a custom tag for JSPs you come across the strangest problems.
For example, I was creating a JSP custom tag for generating JavaScript. The generated code is responsible for filling a JavaScript associative array with values so that they can be used later on in the JSP. Here is the kicker — the JSP can optionally create a variable with the same name before using the tag and fill it with its own values. In that case the custom tag has to append its own values to it instead of creating a new array. This means the the generated code cannot simply declare a new object, since this would erase any of the values that the JSP inserted into it. You have to love requirements sometimes.
After a bit of experimenting I discovered that all the variables (and functions for that matter) for the page was stored in the window object.
if (typeof(window['varName']) == "undefined") {
window['varName'] = new Object();
}
Voila. The above checks to see whether or not the variable has been defined, and creates it if it’s not.
Permalink
Trackback
2008.09.03
Posted in Funny at 3:41 pm by The Rontologist
I love it when I find a new source of humour on the web. I came across The Stonemaker Argument today and had a good laugh.
Permalink
Trackback
« Previous entries