Sign in to follow this  
Cornstalks

33 parameters for a function?! Seriously!?

Recommended Posts

Quote:
Original post by MikeTacular
my boss recently had me make an API change [...]
He agreed but said "the guys" (dunno who they are... his management maybe?) like to use parameters and were pretty stiff about it. He said it was probably so it could work with legacy code and that "the guys" say objects cause portability issues.
Is this normal?
Yes. It is quite common that people who take decisions have no flipping clue what they are talking about. However, they are still the ones who decide.

This is where corporate branding and FUD have their base. It is what influencer marketing aims at too. Big companies have been successful selling inferior goods and services with that strategy for decades.

In other words, executives will rather spend 250,000 for something that works less efficient and has higher maintenance cost than something they could get for free or nearly so based on the fact that some guy they play golf with says it's what people use, or because it has a big name.

It looks like your boss has a great career as executive ahead. :-)

Share this post


Link to post
Share on other sites
d000hg    1199
Quote:
Original post by samoth
Quote:
Original post by MikeTacular
my boss recently had me make an API change [...]
He agreed but said "the guys" (dunno who they are... his management maybe?) like to use parameters and were pretty stiff about it. He said it was probably so it could work with legacy code and that "the guys" say objects cause portability issues.
Is this normal?
Yes. It is quite common that people who take decisions have no flipping clue what they are talking about. However, they are still the ones who decide.

This is where corporate branding and FUD have their base. It is what influencer marketing aims at too. Big companies have been successful selling inferior goods and services with that strategy for decades.

In other words, executives will rather spend 250,000 for something that works less efficient and has higher maintenance cost than something they could get for free or nearly so based on the fact that some guy they play golf with says it's what people use, or because it has a big name.

It looks like your boss has a great career as executive ahead. :-)
It's also common for technical people to think their bosses are stupid because they are less technically able than they are. This is logically ridiculous. Your boss has to manage people, organise schedules and budgets, keep clients happy etc, which all take time and require knowledge and skills. Only a very special person can do all that and still know as much as his development team. In fact even if he is a coder by trade, a good boss would be hiring coders who were better than him.
Expecting your boss to be at the same level as you on your specialty is unreasonable. Programmers need to learn not to feel superior to people who aren't great programmers, and how to communicate with less technical people.

Share this post


Link to post
Share on other sites
M2tM    948
Yeah, I'm glad this resolved properly into a refactoring of the function into an object. It is exactly the type of thing that shouldn't be a single function.

Share this post


Link to post
Share on other sites
trzy    100
Quote:
Original post by RivieraKid
i dont even think stuffing 33 parameters into a struct/class is a good solution. What a mess.


I agree. Unless the function is called very frequently (in which case maintaining a struct with configuration information would be far more efficient), creating another layer of abstraction seems rather pointless.

Is this,


params.a = 1.0;
...
params.z = 26.0;
Init(&params);






really any better than


Init(1.0,
...,
26.0);






?

I submit that it is not. The only advantage of the first is that it is clear which argument corresponds to which parameter. It's not immediately obvious from the latter, although that could be rectified with comments.


Init(1.0, // a
...
26.0); // z


Share this post


Link to post
Share on other sites
benryves    1999
Quote:
Original post by trzy
Quote:
Original post by RivieraKid
i dont even think stuffing 33 parameters into a struct/class is a good solution. What a mess.


I agree. Unless the function is called very frequently (in which case maintaining a struct with configuration information would be far more efficient), creating another layer of abstraction seems rather pointless.
Or, indeed, if a lot of parameters have default values - but this is better resolved with named/optional parameters.

Share this post


Link to post
Share on other sites
polymorphed    272
Quote:
Original post by trzy
Is this,

*** Source Snippet Removed ***

really any better than

*** Source Snippet Removed ***

?


When you make a function which takes 33 parameters, you (I'm taking the role of the processor here, bear with me) have to fill in each and every one of those variables on to the stack when the function is called. This comes at a cost, especially if the function is called a lot. By passing a reference/pointer instead of 33 parameters, you'll only need 4 bytes on the stack, as opposed to 132.

Share this post


Link to post
Share on other sites
Way Walker    745
Quote:
Original post by RivieraKid
i dont even think stuffing 33 parameters into a struct/class is a good solution. What a mess.


Right, it probably wouldn't help matters to just dump all the parameters in one struct. What you'd want to do is group similar parameters into different structs to make the mess of 33 parameters conceptually simpler to someone reading the code. It'd make it faster to locate the parameter of interest.

And, looking at the bit that was posted, I'd probably have to be tearing apart an object to pass it to that function.


namespace Application {
struct Application {
ApplicationType type;
std::string version;
};
}



namespace Asset {
struct Video {
int width;
int height;
float duration;
std::string frameRate;

std::string dataRate;
Asset::VideoBitRateMode videoBitRateMode;

std::string codec;
};


struct Audio {
std::string dataRate;
Asset::AudioBitRateMode audioBitRateMode;

std::string codec;
};


struct Asset {
std::string shortName;
std::string description;
std::string authorName;

std::string fileName;
std::string fileType;

Video video;
Audio audio;
};


struct Paths {
std::string asset;
std::string shortPreview;
std::string mainThumbnail;
std::string summaryThumbnails;
};
}



void myUploadAsset(
const Application::Application& application,
const Asset::Paths& pathTo,
const Asset::Asset& asset,
const Asset::Asset& source,
const std::string& presetXML,
const std::string& filter,
bool (*progressCallback)(float)
) {
uploadAsset(
applicaiton.type, application.version,
pathTo.asset, pathTo.shortPreview,
pathTo.mainThumbnail, pathTo.summaryThumbnails,
asset.shortName, asset.description,
asset.video.frameRate, asset.video.width, asset.video.height,
asset.audio.dateRate, asset.video.dataRate,
asset.audio.bitRateMode, asset.video.bitRateMode,
asset.video.duration,
presetXML,
asset.fileType,
filter,
asset.audio.codec, asset.video.codec,
asset.authorName,
source.fileName, source.fileType,
source.audio.codec, source.video.codec,
source.video.frameRate, source.video.width, source.video.height,
source.audio.dataRate, source.video.dataRate,
source.audio.bitRateMode, source.video.bitRateMode,
progressCallback
);
}

Share this post


Link to post
Share on other sites
trzy    100
Quote:
Original post by polymorphed
When you make a function which takes 33 parameters, you (I'm taking the role of the processor here, bear with me) have to fill in each and every one of those variables on to the stack when the function is called. This comes at a cost, especially if the function is called a lot. By passing a reference/pointer instead of 33 parameters, you'll only need 4 bytes on the stack, as opposed to 132.


I mentioned that it doesn't matter for functions which are called infrequently (such as initialization functions, which may be called once or only a few times per program session.) In that case, pushing things on the stack is no big deal. If you had to set up all the parameters in a struct each time, you wouldn't save anything, because you would still be making just as many memory writes as pushing everything on the stack.

Share this post


Link to post
Share on other sites
Dmytry    1151
Quote:
Original post by MikeTacular
....

its actually fairy common to pass tens or hundreds of arguments to function.... that's called OOP and arguments are called "members" or "fields" :P It also has nicer syntax than just listing them in the (,,,,) list.
(joking of course.)

More seriously though. I never seen a function taking many arguments, which are so unrelated as that you couldn't group them into classes. Example, a function that takes 2 3-d vectors... of course you should make 3d vector a class.

[Edited by - Dmytry on January 11, 2009 1:49:40 PM]

Share this post


Link to post
Share on other sites
Cornstalks    7030
Quote:
Original post by ToohrVyk
I once generated a function with 81 parameters—each parameter was a distinct Su Doku cell, and I needed to manipulate them by name.
I don't get why you had to use 81 parameters. Seems like there would be a cleaner, less error prone solution.

Quote:
Original post by trzy
*snip*
I beg to differ. The difference is readability. Intellisense makes it really easy to build a struct like that, because each time you access a member with '.', you get a nice, readable list of members. If you're trying to call the function with 34 arguments, when you write '(' intellisense is going to give you a boat load of parameters, many of which look like this: const std::string& parameterName. The const std::string& kills readability. And it's easy to forget which parameter your on, so one mistake can cause a huge ripple effect. You also suggest using comments to clarify the 34 parameters. Not only does that take time, but it requires the programmer to know exactly what all 34 parameters are and (here's the hard part) exactly what order they're in. With a struct, you can fill them in whatever order you like, and the member names are named reasonably so you shouldn't need any comments. Hence increasing productivity and readability.

Quote:
Original post by Way Walker
*snip*
I actually really like that solution. Cramming all 34 parameters into an object does seem messy. Splitting them up into coherent objects like that makes a lot of sense. I'll suggest something like that.

Quote:
Original post by d000hg
It's also common for technical people to think their bosses are stupid because they are less technically able than they are. This is logically ridiculous. Your boss has to manage people, organise schedules and budgets, keep clients happy etc, which all take time and require knowledge and skills. Only a very special person can do all that and still know as much as his development team. In fact even if he is a coder by trade, a good boss would be hiring coders who were better than him.
Expecting your boss to be at the same level as you on your specialty is unreasonable. Programmers need to learn not to feel superior to people who aren't great programmers, and how to communicate with less technical people.
I hope you weren't targeting me in that. My boss is a programmer/software engineer and mostly works in Java. The reason the function grew to 34 parameters is because it started out with about 5 parameters (we hardcoded the rest of the values into the function while testing), but when we removed those hardcoded testing values, we had to suddenly expand the function to take all the values we needed. It just hadn't been planned out (not that that's a good excuse, we should have planned it out long ago). Anyway, that's how it evolved from a 5ish parameter function into a 34 parameter beast.

Share this post


Link to post
Share on other sites
Rydinare    487
Seriously. That's a hazard which begs for redesign. Neither solution is really great. You simply went from an F up to a D-. What you really need is to componentize the different facilities... but if it took that much just to get them away from a 33 argument parameter list, yikes. Good luck.

Share this post


Link to post
Share on other sites
EvilNando    96
Quote:
Original post by d000hg
Quote:
Original post by MikeTacular
I just had a nice little chat with my boss and a coworker. My boss is all for using a struct/class, but my coworker isn't. He likes having a lot of parameters.
Who cares, it is your task and your boss is happy, just do it your way and if the coworker really whinges, tell him he's welcome to add an override that takes the parameters individually.

But, 33 is nothing.


Thank you sir , never laughed so hard in my life


seriously

Share this post


Link to post
Share on other sites
trzy    100
Quote:
Original post by MikeTacular
Quote:
Original post by trzy
*snip*
I beg to differ. The difference is readability. Intellisense makes it really easy to build a struct like that, because each time you access a member with '.', you get a nice, readable list of members. If you're trying to call the function with 34 arguments, when you write '(' intellisense is going to give you a boat load of parameters, many of which look like this: const std::string& parameterName. The const std::string& kills readability. And it's easy to forget which parameter your on, so one mistake can cause a huge ripple effect. You also suggest using comments to clarify the 34 parameters. Not only does that take time, but it requires the programmer to know exactly what all 34 parameters are and (here's the hard part) exactly what order they're in. With a struct, you can fill them in whatever order you like, and the member names are named reasonably so you shouldn't need any comments. Hence increasing productivity and readability.


There is certainly no performance advantage either way. Depending on how the rest of the API is designed, an information struct (or multiple structs) might make a lot of sense. But I could envision scenarios where it wouldn't be worth the hassle to create a struct (or God forbid, a class) where people would have to take extra care to make sure they initialized every member properly.

Personally, I don't use IDEs very often, so I can't rely on IntelliSense or similar features.

Share this post


Link to post
Share on other sites
Scourage    1198
Bottom line up front: 33 parameters for a function means you have a problem

1. Go out right now and buy "Refactoring" by Martin Fowler.
2. Read it over and over again until you understand that 33 parameters is a serious "code stink".
3. Kick your boss in the nuts (optional).

If this is production code that you're working on, that means you will need to maintain it and support (hopefully) many customers using the code. Good luck finding bugs in code where that kind of function call is acceptable. I imagine a customer would not be that happy to use that function either. Just that function alone makes me nervous about the rest of your code base.

Cheers,

Bob

Share this post


Link to post
Share on other sites
d000hg    1199
Quote:
Original post by MikeTacular
Quote:
Original post by d000hg
It's also common for technical people to think their bosses are stupid because they are less technically able than they are. This is logically ridiculous. Your boss has to manage people, organise schedules and budgets, keep clients happy etc, which all take time and require knowledge and skills. Only a very special person can do all that and still know as much as his development team. In fact even if he is a coder by trade, a good boss would be hiring coders who were better than him.
Expecting your boss to be at the same level as you on your specialty is unreasonable. Programmers need to learn not to feel superior to people who aren't great programmers, and how to communicate with less technical people.
I hope you weren't targeting me in that. My boss is a programmer/software engineer and mostly works in Java.
Nope not aimed at anyone in particular, just a response to the "bosses are no use if they aren't great programmers" school of thought. Even a boss who was once a full-time programmer can't help but fall behind as he spends less and less time working on software development though, which is what normally happens - and this is doesn't mean he's no longer able to be a good boss.

Share this post


Link to post
Share on other sites
d000hg    1199
Quote:
Original post by trzy
There is certainly no performance advantage either way. Depending on how the rest of the API is designed, an information struct (or multiple structs) might make a lot of sense. But I could envision scenarios where it wouldn't be worth the hassle to create a struct (or God forbid, a class) where people would have to take extra care to make sure they initialized every member properly.
You can set defaults if you have a class, or you can have a solution where you get an error trying to use an uninitialized object.

However, the simple struct approach is very prevalent in Java web applications, where these utility classes are called Java Beans... basically classes with only simple setter/getter methods. They are often mapped to databases and provide a convenient way to pass data around.

Share this post


Link to post
Share on other sites
Zahlman    1682
A useful technique for this sort of thing.

Quote:
Original post by ToohrVyk
I once generated a function with 81 parameters—each parameter was a distinct Su Doku cell, and I needed to manipulate them by name.


LOL... ?

Quote:
Original post by Benjamin Heath
o_O

For purely educational purposes, why would you do such a thing?


At a guess, because adding even a user-defined default constructor to a struct makes it no longer qualify as POD per the Standard (I consider this kind of a WTF in itself - of course C doesn't have these options, but there's no reason they ought to have any power to impact the memory layout), and POD-ness is useful for interacting with some older code, doing pointer hackery (especially with I/O), etc.

Quote:
Original post by d000hg
It's also common for technical people to think their bosses are stupid because they are less technically able than they are.


I don't think this is what actually happens. More likely, people think their bosses harbour dangerous delusions about their technical expertise, and are frustrated that said bosses are unwilling to defer technical decisions to people who were hired specifically for that expertise.

Quote:
Original post by polymorphed
When you make a function which takes 33 parameters, you (I'm taking the role of the processor here, bear with me) have to fill in each and every one of those variables on to the stack when the function is called. This comes at a cost, especially if the function is called a lot. By passing a reference/pointer instead of 33 parameters, you'll only need 4 bytes on the stack, as opposed to 132.


But if you don't already have the object, a reference/pointer to which you would like to pass, then you'll have to construct it, and thus put the data on the stack anyway. The complexity has to go somewhere.

Share this post


Link to post
Share on other sites
curtmax_0    100
Quote:
Original post by RivieraKid
i dont even think stuffing 33 parameters into a struct/class is a good solution. What a mess.


Well, 33 might be a bit much, but some functions do need alot of information. D3D does this very often. Functions take structs with data in them (in addition to other parameters). If all the data were in function parameters it'd be something like 80 parameters or such!

The structs also 'sort of' emulate named parameters because you can leave ones blank you don't care about.

Quote:
Original post by Zahlman
A useful technique for this sort of thing.


Very interesting. Seems like alot of setup just to do that though.

Share this post


Link to post
Share on other sites
Nytegard    839
As much as many of you are laughing at why anyone would use a 33 parameter function, honestly, it's one of the more sain pieces of programming I've seen. Sadly, university tends to teach more of a theoretical approach, and not a realistic approach, and reality tends to be that even professional companies have code that's garbage at best. I've seen functions with over 1000 parameters, I've seen a function which converts a function to a string to be converted to an int, etc. And sadly, this isn't at just one company, and nor is it just a random programmer. Some of the worst code I've seen that could even make it to thedailywtf has come from people who've graduated from Stanford, MIT, and Carnegie Mellon.

Sadly, in almost every job I've ever been in, the idea has been more that "If it works, you can always refactor it later when you have time, but get the next thing out today." Today means adding to the mess, and you'll never have a later in which you can fix what should have never been written in the first place.

Share this post


Link to post
Share on other sites
d000hg    1199
"Crap programmers write crap code all the time" isn't an excuse. Many people out there have no good skills and just throw out anything that works. It's true you can go overboard on the computer science part but this is something quite trivial, if it wasn't for the multiple API issue I'd expect any decent coder to make this change without thinking they need to discuss it. Any professional coder should be automatically writing reasonable code to start with, of course it often won't be perfect but there are degrees of quality and anyone who likes to think themselves a programmer (rather than someone who hacks code) should be a few rungs higher up the ladder!

Share this post


Link to post
Share on other sites
Mithrandir    607
Quote:
Original post by MikeTacular
Would you ever write or use a function that takes 33 parameters? And how can you help someone see that 33 is just too many? Even though all 33 are absolutely needed, how can you help them see that passing a single object containing all the needed info would be better? Background info/story:

I'm at work ("working," of course) and my boss recently had me make an API change to an SDK we're making. He does the Java version of the SDK, and I do the C++. Basically he (and others) decide how it's going to be, then he does it in Java and gives it to me to turn into C++. He just gave me a function that takes 33 arguments. 33! Most of them are strings, but there is a healthy(?) mix of ints, floats, and enums, and they are in no particular order. It's horribly confusing. And this isn't the first time. There was a Java function they had me clean up that probably had about 30ish parameters (I cut it in half but still, why?). I suggested making a struct or class that contains all the necessary parameters that we pass into the function. That way we can get all 33 parameters we need but only pass 1 object to the function. He agreed but said "the guys" (dunno who they are... his management maybe?) like to use parameters and were pretty stiff about it. He said it was probably so it could work with legacy code and that "the guys" say objects cause portability issues.

Ummm... portability issues? I'm not buying that one. The object would be 100% portable. And it's a static library so there aren't the DLL issues either. And I'm definitely not buying the legacy code thing, since this is a brand new SDK we're making. There is no legacy code. Anyway, it's hard to use (and maintain) a function with 33 arguments, especially when they usually change every week when the API gets updated. I like my boss; I don't blame him for the parameter mess. He's a smart guy. So I tell myself I blame his boss.

Is this normal? Do most work places do this (by "this" I mean use a ridiculous amount of arguments for a function)?


At my last company our entire product was littered with this stuff.

We actually had a family of over 40 function overloads which all ended up calling the same function in the end, and that one function took over 70 parameters. I think the average parameters per overload was somewhere around 50 or so, and each one was just various combinations of those 70 parameters.


This arose because of the sloppy management style they had. There were no code reviews, and developers were literally treated like assembly line workers. When I finally left, we had 20 unpaid interns and 9 full time developers. The devs were encouraged to imitate previous issues rather than fully understand the code.

So every time a function needed another option, they would get an intern to just add a new parameter or two to the existing code, and be done with it. Refactoring was considered wasteful and time-consuming.

Right before I left I laid out a new architecture (as I was eventually promoted to lead architect) for them to use that would avoid all this, but I was told that what they have already works, so they didn't see the point in going over the the new system.

Just one of millions of reasons why I left.

Share this post


Link to post
Share on other sites
Mithrandir    607
I'd also like to point out that code written by outsourcing companies frequently looks like this too.

They either

A) don't care about the maintainability of the code because they are a one time contract, or

B) intentionally sabotage the maintainability of the code so you'll have to go back to them when you need changes or fixes.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this