I’m busy re-writing vast swaths of my low-level code in order to support a new database schema. One of the classes that I wrote that got used all over the place is called “Playlist”, and its various subclasses. Playlist has an abstract clone() method. I almost never write clone() methods, so somebody must have asked for it. And I think I know who now.
As part of this re-write, I using Eclipse’s “Find References” function a lot to see if some method needs to be re-written or can just be deleted. While doing that, I found an interesting bit of code in an obscure part of the system that I’ve never looked at before:
private Playlist doClonePlaylist(Playlist playlist) {
Playlist clonePlaylist = null;
// need a special clone
// as the playlist cone function
// doesn't copy the playlist
// external id which we really need
if (playlist != null) {
clonePlaylist = (Playlist)playlist.clone();
clonePlaylist.setExternalID(playlist.getExternalID());
}
return clonePlaylist;
}
If the guy who’d written this bit of cruft still worked here, I’d want to ask him why the fuck he didn’t just tell me that I’d left one parameter out of my clone() method, or even better, fix it himself? Ideally, I’d want to ask him while dangling him over the edge of a multistory building, but that isn’t going to happen.
// I have eaten the plums
// that were in the icebox
// as the playlist cone function
// doesn’t copy the playlist
// external id which we really need
There. Much better.
Yeah, I’m always amazed at programmers’ unwillingness to fix or critique “someone else’s code.” I don’t know whether to attribute it to lack of confidence, a perceived lack of authority, or just plain lack of intelligence.
If any of my code is wrong, fix it. Please.
>If any of my code is wrong, fix it. Please.
I don’t agree with this. Sometimes there’s a reason I did something one way, and if you don’t really know the context, you could break it.
An example from a few years ago: We had a base singleton class, and there were some quirks in the constructor. One of our senior developers got sick of it and fixed them, and as a side effect made the class un-thread-safe. He didn’t tell anyone he’d made the change, and it got into the field that way and required a hotfix when it was found. If he’d walked down the hall and asked the original writer, they’d have told him why it was the way it was.
Yeah, but up until this guy got fired in Christmas 2006, I sat about 5 cubes away from him and he could have said “Is there a reason why your clone method doesn’t preserve externalID”, said “I need you to fix your clone method so it preserves externalID”, or written up a PCR at any time.