- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- 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
max( Frist, Second, NaN, FIleNotFound)
Admin
Mariette confirmed that attempts to fix the function broke many things in the application, so she did the only thing she could do:
... fixed them.
FTFY
Admin
I don't get why she didn't remove the function altogether. You can't miss a place where it's used as the compiler will flag all invocations with a too many arguments error...
Admin
Because than she would be the one responsible for causing a hundred and one errors in areas of code that previously worked. Areas of code written by other people. People that would come shout at her for breaking their code.
Admin
There could be other functions in the code base called max, that take 2parameters, that do something else; so changing the signature of this function could then cause other problems. Maybe that's what the other parameters are for - to control which max function gets called? (like the int parameter that gets added to operator++)
Admin
'There could be other functions in the code base called max, that take 2parameters, that do something else'
If they do something else they should probably be named differently.
As for removing unused parameters. You can assign a default value to parameters, right? Then you can take a phased approach where you warn on the usage of max with values c and d. That would let you fix occurences one by one and give visibility to other devs that use max incorrectly
Admin
so she promoted it out of the way. how corporate of her.
Admin
Did they not use version control? I'd have been very tempted to view the history of this file to see how it ended up this way, hopefully with some commit messages or linked tasks or whatever to help understand.
Of course the chances are that nothing would make any sense, but at least I'd want to try to gain whatever little insight is available.
Admin
There are people who enjoy their work, and those who do the minimum possible, sadly.
Admin
I've taken that approach before - the problem that invariably results is either a) people don't even notice the deprecation warning because it's just a handful among the thousands that go unheeded, or b) in an attempt to prevent a) someone turned on the "Warnings as Errors" flag long ago (which is probably what prompted the no-op code in the first place), and then you're right back where you started with this change spewing out a bunch of build errors.
And sometimes c) to avoid the problem with b) you disable that specific warning, which just leads you right back to a).
In short: deprecated code is just a can to keep kicking down the road until it goes off a cliff.
Admin
If there are other functions named max with 2 int parameters that do something else, they deserve all problems arising from the removal of the 4 parameter function...
Joke aside: I didn't even envisage the possibility of a 2 parameter overload that does something else. I must be lucky to work on a code base that doesn't have that kind of WTFs.
Admin
At least if you use a "#pragma unused", it'll be more informative than "// Do nothing".
But as Greg was trying to say, just comment it out entirely. Then try to build and you should see all the places where it was (ab-) used. If you're lucky, you might even get a clue about what was going on in the deranged mind of whoever 💩ed this very stinky code.
Admin
Once again, the issue isn't how to fix this in a technologically sound way.
The issue is ALWAYS how to manage the horrendous consequences of any sensible fix across a large craptacular test-free codebase involving lots of other people and departments all of whom are too overworked to do anything but the cheapest most momentarily expedient thing. They create more new technological debt in a day than could be paid down in a month. Day in and day out.
That is the hard problem of 21st Century IT. The rest is just details.
Admin
Because spending large chunks of time fixing something that's just bad but not actually broken is often a poor use of time.
Admin
Hard to see why fixing this would use a large chunk of time (VS's built in refactoring commands could likely do it), but even if it did, anything that makes code easier to maintain is never a poor use a time, unless there's good reason to suppose nobody will be maintaining it at all much longer.
Admin
You do work in this horrendous industry? What you say is common sense, and so needs to be hunted down and stamped out all costs. All the more since all the Agile nonsense caught on, where does making stuff more maintainable and less buggy actually fit into anyone's damned "sprint"? I'd be willing to bet that mentioning "technical debt" in a stand-up in most places is tantamount to walking the plank these days.
WTFGuy already summed it up better than I can.
Admin
I’d assume that there are calls like max (x, y, a=2, b=3) where removing the last two parameters would remove the side effects.
Admin
It's still the right thing to do, though.
If nobody notices because there's a blizzard of other warnings, then you already have a more basic problem -- your fellow coders are slatterns.
And I agree - at a bare minimum, the fix is to remove the parameter names (thus stopping the complaints from the compiler), add a pragma, and
Admin
You do realise that we're not allowed to be rude about Agile, don't you?
Down my local pub, we are enlightened people. Nobody complains when you discuss politics, religion, or watching octopus porn on Anime channels.
But Agile? Dear me. Twitter storms will ensue. It's gone beyond a religion and into the hyperspace of doubt-free insanity.
Admin
... and (damn this stupid keyboard) try a simple refactor in your IDE of choice. Ten places to fix? Do that. A thousand? Perhaps not. But probably worth sending a polite email to your cow-orkers.
Admin
Where I work, we do something that's called Agile, accompanied by some oddments intended to accommodate the reality lurking behind the Agile myth that complexity of tasks is a good measure of the time they take.(1)
My team has an on-going (part-time) project to pay down technical debt and functional lacks in our corner of the product.
(1) If all tasks are similar in nature, say yet another web view of some data thingy, then the complexity of one task is broadly similar to the next one and the previous one, and in any event the time required for a certain amount of complexity is very similar, so it's useful to say how much complexity you can do in a single sprint.
If, on the other hand, the tasks range from deep-juju hairy-looking code at one extreme (very brain-cell consuming even if it doesn't take a lot of time) down to carrying out benchmarking parts of the product at the other (low complexity, but very time-consuming), then relying on a complexity measure to schedule work into sprints is nonsense.
Yeah, yeah, I know, benchmarking is so ... twentieth century ... but sometimes we need to know what the product is capable of.
And of course "diagnose the deep juju that's gone wrong that causes this problem" is almost impossible to schedule on any project management system, but it's even worse for Agile, where neither the time nor the complexity can be predicted.
Or maybe we've misunderstood what a "Story Point" is supposed to actually measure. That's always a possibility.
Admin
You're looking at Agile the wrong way. Think Sushi!
(Seriously.) The loons appear to have given up on cute naming of this type, but I vividly remember, back in the day, when Sashimi was the Next Big Agile Thing.
Apparently it's to do with "delivering code, one thin slice at a time." My source goes into great detail about "decomposing software."
It's not quite so eloquent on the subject of "decomposing raw fish."
Admin
Just a note "deprecated" dos NOT mean "obsolete", "to be removed", or anything beyond there exists a recommendation to (at least under certain circumstances) use something else.
Admin
I've worked at several shops with their own idea what they are. Some just use them as a subsitute for a time estimate, essentially computing the n of fibb(n). Others have some kind of Dutch auction where junior developers tend to estimate low, and experienced developers high, and the senior developer (me) is expected to do the job for the junior developer's bid. That's like taking a car to an authorised dealer for repair and expecting it at back-street prices.
What I believe they should be used for is the importance to the customer (i.e. the person who told the story). The complexity or amount of time it takes is not relevant: we have other measures to estimate those. It should be purely a measure of importance.
The other big fallacy is that sprints should be two weeks long. There is no particular reason for it to be so.
Now excuse me while I go for my morning jog. Today I am aiming to run a marathon, by running it in forty-three 100-metre sprints. You see, it is quicker that way.
Admin
If I would stumble upon this, I would just fix it by removing c and d. Since they aren't used, removing them don't change anything. Then just use refactoring tool and remove 3rd and 4th parameter from every call to that function. Doesn't matter if there are a million changes to code base. Resulting codebase would quite obviously be better than the original.
Admin
I enjoy my work. Which is exactly why I want to keep doing it.
And in a lot of workplaces being the guy that breaks untold thousands of function calls across a giant codebase is a good way to not be doing it any more. And being the guy that has to tell the boss he spent untold hours fixing what ain't strictly broken is an even better one.
Admin
Surely she renamed the function? She could call this maxIgnoringCandD. If that name seems long, good - hopefully it will prompt everyone to look for a max function that does what you expect.
Admin
"Or maybe we've misunderstood what a "Story Point" is supposed to actually measure. That's always a possibility."
My understanding is it's supposed to measure effort, not complexity.
https://www.mountaingoatsoftware.com/blog/its-effort-not-complexity
Admin
The dictionary disagrees with you, but that may be because common usage has overtaken the traditional definition in our sphere. Always good to google before getting pompous.
Admin
There's a good quick-read blog post on story points here: https://www.scrum.org/resources/blog/why-do-we-use-story-points-estimating . In short, as @nasch also points out, they are indeed a measure of effort--not complexity, not time, and also not importance.
The Scrum way of measuring importance is to choose to do the thing, by adding the user story into a sprint. Partly because what's important can change over time.
If you're using Scrum, then at the end of those forty-three sprints you may just find yourself in a very different place from where you expected to end up when you started.
Admin
MSVC gives you a macro-defined max(a,b) function:
#define max(a,b) (((a) > (b)) ? (a) : (b)), in minwindef.h. It sounds like whoever came up with this didn't trust it (NIH), so they had to make their own implementation, which then needed a different signature to be distinguishable from the macro definition.Of course there's a huge difference here. If a == b, the macro returns b, whereas the four-parameter version returns a!
On a side note, if they were using VS and wanted to work around the macro definition, they can undef it, or use #define NOMINMAX
Admin
This could make sense if for some reason they are passing function pointers around. Suppose you had sum(a, b, c, d){ return a+b+c+d; } foo(i,f){ return f(i,i+1,i+2,i+3); } foo(3,max); foo(4,sum);
Or something like that. Now you need to take in 4 parameters because another function uses them.
Admin
Maybe their coding standard requires that they cannot use the preprocessor beyond #include.