July 12, 2008

compacting java code, and style

An un-named library included this joyous piece of sample code (please ignore the gratutious style violations -- they were clearly C++ coders):

public String[] getPrefixes(String prefix) {
Vector<string> v = new Vector<string>();
for(String s : map.keySet())
{
if(s.startsWith(prefix) && !s.equals(prefix))
{
v.add(s);
}
}

if(v.size() > 0)
{
return (String[])v.toArray(new String[0]);
}
else
{
return null;
}
}


I thought, "This is a place where scala can shine. Let's try to remove the 99% of this code that's just boilerplate." Here's what I came up with:

def getPrefixes(prefix: String): Array[String] = {
val keyList = (for (val key <- map.keys if key.startsWith(prefix) && (key != prefix)) yield key).toList
if (keyList.size > 0) keyList.toArray else null
}


But wait. That's really terse, but doesn't look awesome. The first line is too long to completely absorb when skimming, and the last line is questionable style -- I wanted a ruby-style "keyList.toArray unless keyList.empty?" but scala uses inline if/else for the "a ? b : c" operator so it looks weird. Second try, let's use matching instead:

def getPrefixes(prefix: String): Array[String] = {
(for (val key <- map.keys if key.startsWith(prefix) && (key != prefix)) yield key).toList match {
case Nil => null
case list => list.toArray
}
}


That works. The "(...).toList" construct still bothers me a bit though. Is there some better way to do that?

4 comments:

jherber said...

of course, you could look at this another way:

def prefixIter = map.keys.filter(prefix==_)

then you just perform you ops on those prefixes:

prefixIter.foreach( println _ )

Jorge Ortiz said...

What's wrong with returning an empty array? Dealing with nulls is a pain anyway.


Then either of:

(for (key <- map.keys if key startsWith prefix && key != prefix) yield key).toArray

or

map.keys.filter(key => key startsWith prefix && key != prefix).toArray

works.

sadie said...
This comment has been removed by the author.
sadie said...

The more Scalish way is not to return an array at all, but just a sequence - and in the case of no elements, an empty sequence.

def getPrefixes(prefix: String): Seq[String] =
map.keys.filter(key => key.startsWith(prefix) && key != prefix)

Then if the client code cares about the format, it can call toArray, toList, etc