- Feature Articles
- CodeSOD
- Error'd
- Forums
-
Other Articles
- Random Article
- Other Series
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
List<Integer> partNumber = new ArrayList<Integer>(); int frist = 1; partNumber.Add(new Integer(frist));
return partNumber[0]
Admin
This one's actually a member of the august group of Hourly WTFs. Happens. All. The. Time.
Admin
So TRWTF is "an partVersion", amirite?
Admin
And that, in a nutshell, is why we will always have these kinds of problems, and HPCs will always have work.
At the start of my career, I worked next to mainframe guys. They didn't write byte-one of code until every single bit of data had been documented, every single interaction had been spec'd out in excruciating detail and every possible scenario was documented for testing. Did it take forever to get anything built? Sure. But their systems never crashed, at least not the ways the ones we slammed together in death marches did. Maybe they were onto something...
Admin
Never crashed, until Y2K required the death march only to push it off to 2037.
Admin
StringUtils is obviously org.apache.commons.lang.StringUtils. :P
Admin
Looks like searchString can contain multiple version eg, PPPPP-1-3-7 would return the list 1,3,7. The writer is assuming searchString should be a part number, but there's nothing in what's been presented to confirm this. It seems a bit of a weird way to pass around multiple versions, but I guess some other part of the system builds the string in that format, so this has to parse it. Maybe the front end lets you select a subset from a list of versions of a given part, and someone thought it was a great idea to pass them to the back end like this?!? Never a good idea to change behaviour like that unless you're sure you understand it, but I'm sure the unit tests would have caught it :-)
Admin
Bit too familiar with this sort of magic-numbering in "unique" "identifier" strings. Of course over the past few years I've learned that "it is always in this format" is actually spelt with a silent ", except when it isn't".
Admin
Death marches generally mean project management has failed. I don't remember doing death marches when preparing for Y2K on our mainframe applications. Management had seen it coming and planned accordingly several years before. As Snoofle pointed out mainframe applications generally were rock solid. At least a lot of those I worked on were. I've worked on projects where the specs were written in APL and tested on the computer before a single line of code was written in the final coding language (Fortran, COBOL, PL/I whatever). Other projects were specc'ed using Z, all of which IMO really helped the reliability. I'm not saying everything was perfect in the mainframe world, far from it but compared to a lot of stuff I saw (and interfaced with) on the Unix and PC side, mainframe s/w was pretty damn good.
Admin
Based on the part number format, TE/AMP was the company? TRWTF is that every time you Google one of their part numbers Google thinks you're having trouble doing math and are asking for help in subtracting 1 from a big number.
Admin
I had a similar experience while working as a contractor for a larger Enterprise during the last decade. One team in particular had a rigid peer-review process for code. It's too detailed for a simple post like this. The end result, as strange as it may seem to hear someone say this, whatever was checked into the main branch in the source repository: worked, every time. Yes, the process was a bit slower but "it works" is hard to argue against.
Admin
80 hours a week is not even "half time"..... What is wrong with you people? Have you no work ethic???? <<ducking and running>>
Admin
A typical mainframe programmer fully expects that when he is fifty years old he will be responsible for maintaining code that he himself wrote when he was twenty-five.
Admin
Well actually, I once had a manager who said "we are going to follow process." - decide what you are going to do, how you are going to do it, review everything, you know the full process. Upshot was our group had a bad reputation among the rest of the company for being slackers as we were NOT part of the death march. We had free Saturdays, worked leisurely 50 hour weeks. And we delivered on-time or ahead of schedule with quality. Following process actually lets you get the work done in less time with higher quality. Or as we used to say "the sooner the coding starts, the longer the project will take."
Admin
Death marches ALWAYS mean project management has failed. No exceptions. Lots of companies are very, very bad at project management, and most of the rest are merely not good at it, but in all cases, a death march means that too much work was placed into too little time.
Admin
First of all, there's nothing complex enough about the original requirement (I know, I know, we don't know what it was, but still and all) that this function couldn't have been continually refactored as the requirement was clarified.
And second of all, even the original requirement (I know, I know...), in all its shambolic glory, could almost certainly have been written as a regexp. Which would have been easy to refactor.
I must be having acid flashbacks in my old age. This is literally the first time I have ever said "you should have used a regexp."
Admin
I want to like peer review, I really do, but in practice it's been a farce.
When I was subjected to peer review, the "peer" was always the least capable developer that proceeded to get hung up on local naming and bracket placement. They'd scream about how "unreadable" my code and then churn out spaghetti 10 times worse with 100 times the bugs. Yeah, make the guy who finishes his work for the week by lunch on Monday rewrite and rewrite and rewrite to an ever-changing "standard" policed by the cripple that's been on the same week's worth of work for several months.
Of course, when I had to do peer review, all of that went out the window. Windows/pages/errors rife with misspellings? Fine! Blindly concatenating SQL statements from client input? Fine! Rolling your own date function that's provably wrong 1/3 of the year? Fine! Duplicating half-baked StackOverflow copypasta everywhere? Fine! Stripping license text from licensed components and inviting a lawsuit? Fine! This stuff goes straight into production and then everybody panics when the inevitable results like it's some sort of surprise.
In comparison, this function returning a list is not much of a WTF. There was probably some point where an item listing was formatted like "part-v1-v2-v3" as sort of a shorthand for having three separate listings. This function then works on a normal part listing and the shorthand. If anything, TRWTF is, assuming all callers use only the first item, not fixing the function and correcting the calls. But then we all know how fearful the cobblers posing as programmers can be; they barely got it working as is and frightened of not being able to cobble it back together.
Admin
I have to disagree with the proposed solution.
A function called getPartVersions should return a List of type PartVersion (be that integer or string, or custom). Unless it is expected to return how many versions there are/were - in which case it could return an Integer.
If it is only expected to return the current version identifier, then it should be named getPartVersion
Admin
At the risk of stating the obvious, there should have been a "Part" class with a toString() method and a getVersion() method.
Admin
^^^ THIS.
Not relevant to the OP abortion, but much more important. Code Review is too often seen as a form of micro-detailed willy-waggling, and only in the better type of organisation used as a protection against (possibly quite innocent) mistakes.
These days I'm more inclined to use my "authority" as a code reviewer simply to ask the question: "Have you written a unit/functional test for X?"
It saves a back-and-forth about whether you should use implicit or explicit or faux-Hungarian typing, or some God-Awful "pattern," and it's an easy question to answer.
Admin
Not at all. Clearly it should be derived from a base class that has a default, though occasionally unsatisfactory, toString() method. And getVersion() is ambiguous, because it might refer to the schema -- in this case, we seem to have gone through several business requirements, so I'm not actually joking here -- or to the widget.
I think the best solution here is probably the "dependency injection" bit of SOLID. The dependency is, of course, the arbitrary method used to determine what the "version" might be. Ideally this would be implemented via reflection on the method in question and determined by an XML file, which of course solves the problem of the schema version.
Be sure to implement PrintToWoodenTable()!
And yes. I. Have. Seen. This.
Admin
That's about right.
Now add in a AI that does whatever you want. Augmented Reality. Metaphysical Reality. Reality Reality. Enough urfstuff from the Ancient Ionians to pass around.
Then add in a series of infinitely many booby traps that the code ignorantly stumbled upon as it miraculously runs on a old clone floating in the middle of a maelstrom in the Pacific Ocean.
An earthquake causes a tsunami. Somethings up with this machine where it phases into infinitely discrete packets, causing the hurricane to collapse into itself and shoot all earthbound water in the sun, due to some uncannily spooky, yet empirically deductible "reboot" process.
Admin
Agreed - getPartVersions() is plural, returning a list of all versions for that part makes perfect sense. If you wanted the most recent, add a getPartLatestVersion(), and getPartVersionCount() if that's required.
Admin
Yeah, hyphens/dashes/minus symbols - math operators in general - should be avoided for that and so many other reasons. Use a tilde or number sign or something. Even a lowercase 'r' to indicate a revision.
A better way would be to use a decimal. '0123456.1' Not only does it search better, numerical strings are easy to play with. Only downside is that you need an extra zero or two, so revision 1 isn't counted as revision 10. So your parts number format becomes '0123456.01'. (plus a rule that if you go over 99 revisions, you make a new part number)
Admin
In my experience that's because any attempt to quantify the work, identify requirements or define scope was enthusiastically stamped on as being "fannying about" or "we don't do waterfall anymore" or some other ignorant claptrap. Because "look, it's only a simple little thing". 5 months into the 3 week project, with evil twists and turns and exceptions and edge cases still vomiting out of the increasingly misnamed UAT the programmer will still get the blame.
Dunno if it's appalling project management or actually kind of none at all. The project managers and business analysts just stuck in a tug-circle trying to impress the next layer up and don't bother to do any real work.
Usually the outcome of trying to change a function like that to something more sane is then discovering all the times a part number is NOT actually in that format, or that something else (like assemblies or part lists) is also going through the same functions with only the format pattern to tell them apart.
Needs a reg ex in there so the tears of orphans will help the rotten camel shit stick together.
Admin
The one exception I can think of is when clients come up with last-minute demands and pay through the nose for them. If you knowingly take on a death-march from the start, that's a bit different - as long as the pay is adequate compensation for what you're being asked to do.
Admin
"So your parts number format becomes '0123456.01'. (plus a rule that if you go over 99 revisions, you make a new part number)"
And that's why you shouldn't treat numerical strings as numbers. Or call it a parts number, when it's a parts identifier.
TRWTF here, though, is any company needing an inventory system of that type and not checking what e.g. Toyota do, and copying them. They've got it right by now. Identifiers aren't just unique strings, they're also a code that tells you what the part is, and what it's for.
Admin
There are two ways that work. Pick one, but only one. (I've used both in production; both are entirely viable.)
Yes, you get quite a sizeable database in both cases. Suck it up. The alternatives are much worse.
Admin
Well, way to miss the point. Giving things unique ids isn't hard, but you can make those ids unique and also meaningful. All the massive businesses who have to keep proper inventory control of huge numbers of different parts, like, e.g., car companies, use meaningful codes.
Admin
Where I work, peer reviews do work. Partly because the least capable developer you are talking about doesn't work here, partly because coding standards are written down and there's nothing to argue about.
Admin
Writing them down doesn't preclude them from changing constantly, nor does it mean the people writing them and pushing them on others are following them. It also doesn't guarantee that they're not internally inconsistent, full of holes, or counterproductive (Swapna doesn't understand signed/unsigned so nobody can use one or the other), all of which are opportunities for arguments. That goes double when you willfully conceal these "standards" at interview time, use them to hinder new hires, and cry that product quality is still as poor as it's ever been.
Peer review is never about functionality and always about micromanaging style. Nobody ever says "oh, you should check for a null there because...." or "this component is too tightly coupled to database X." But you'll get back 10 pages of "{ should be on same line as condition" and "SQL should be spelled Sql even in comments." Peer review in a lousy organization works the same way safety reviews work in China. The day the inspector comes, all of the child labor stays home, so the inspector can check off his checklist before going home without asking any questions, and the next day they're back to being chained to a machine with no goggles or hardhats. Then there's an explosion, they find crushed kids in the rubble, and everybody stutters "b-b-but we did inspections so we aren't to blame!"
Admin
"All the massive businesses who have to keep proper inventory control of huge numbers of different parts, like, e.g., car companies, use meaningful codes."
Citation needed. I'm pretty sure Amazon uses meaningless product identifiers, are there aren't many organizations with a more massive product database than that.
Admin
Amazon has a massive unrelated product database. "Real" companies have a massive, multi-level bill of material system, with all sorts of weirdnesses like "##ALT" (from a previous employer) parts (same underlying as-ordered part, but different installed geometry, e.g. flexible tubes). In the end, the meaningful codes end up being fairly important and can lead to significant time-savings.