People in your organization that grow legacy code
zeroturnaround.com
zeroturnaround.com
When I see unwieldy legacy code at Fitbit, I assume it was written by an overworked early employee wearing five different software-engineer hats, two of which they were experienced in. I assume they were working as fast as they could to get their startup to take off before the end of the runway, and that they anticipated having enough money to hire a team of people to clean things up later if they succeeded.
They did succeed, and I'm part of the cleanup crew.
See also http://retrospectivewiki.org/index.php?title=The_Prime_Direc...
Depending on the company, refactoring legacy code is like walking through a minefield to get to a territory that isn't really desired by the higher ups. High cost, low reward.
As for the rule "If you see something wrong fix it properly!" I disagree in some cases. If you are a legacy maintainer, usually something is broken and management/clients want it up ASAP. The proper fix is rarely the fastest. Beyond that, you have to be careful when advising refactoring. If you advise it too much, in cases that don't really need it, management may not take your advice as seriously when there really is a problem.
calendar.getDisplayName(Calendar.DAY_OF_WEEK, Calendar.LONG, Locale.US)
Not that it's necessarily nicer, but generally I'd say duplicating functionality of the standard libraries or frameworks should be avoided if possible.
Assuming that the input string is a formatted date of some sort, then I think this would work, using JodaTime (minus exception handling):
public String getDayOfWeek(String s) {
DateTimeFormatter formatter = DateTimeFormat.forPattern("dd/MM/yyyy");
DateTime dt = formatter.parseDateTime(s);
return dt.dayOfWeek().getAsText();
}Maybe that's a bit of a long shot. Perhaps there's some standard library function -- I don't know Java, but the author seems to assume the reader does throughout the article -- to turn dates/whatever into day number. Maybe his joke is therefore, "OMFG! What a noob!"
In short: I don't get it, either...
"data" is ambiguous, too. I think that perhaps there are a few errors to demonstrate his point.
2: not 0-indexed (there can be good reasons for this, but the context of the function should at least suggest the reason)
3: Doesn't handle errors - should throw IllegalArgumentException, and if should do it in the default handler of the switch. If you suddenly see "???" in the UI, it's not easy to track down to this.
4: If for some valid reason this can't be a library function, it should be an enum, and it should be used throughout the code.
enum DayOfWeek {
MONDAY(1, "Monday"), TUESDAY(2, "Tuesday") ...
public int id;
public String displayName; // these should be private and have getters according to most style-guides.
public DayOfWeek(int id, String displayName) {
this.id = id;
this.displayName = displayName;
}
public static DayOfWeek fromId(int id) {
for (DayOfWeek d : values()) {
if (d.id == id) return d;
}
throw new IllegalArgumentException("Illegal DayOfWeek id passed: " + id);
}
}FTFY
String[] names = { "UNUSED", "Monday", "Tuesday", ... }
try {
return names[i];
} catch(ArrayIndexOutOfBoundsException uhoh) {
return "???";
}
Look, at least it's not a pile of @Inject DayOfWeekLookupAdapter dayOfWeekLookupAdapter with attendant 100kB Spring XML config, of which the unit tests will have their very own special cut and pasted copy. No, I'm not bitter at all.
We have all those people where I work and I have learned to know what questions to ask each of them to make sure they did the right things.
Quality cross group code reviews help identify this stuff fairly fast.
Informal design sketch review, peer code review, pair programming to mentor and induct. All processes that could be mandated by the manager, but work better from peers than down from on high.