How bad is a kludge?

[George10]

This is a STYLE question not a technical question, the code provided is merely to give an idea of what is going on and the level at which I am currently writing code at.

I am teaching myself C++ and am trying my hand at some of the problems posed by Blitz Coder: http://www.cplusplus.com/forum/articles/12974/ ) and borrowed some code from the tutorial on basic i/o ( http://www.cplusplus.com/doc/tutorial/basic_io/ ) to make my user input more stable.

The problem I am running into is whether or not to kludge.

For example the first problem calls for the user to enter a number, but what if they don't enter a number at all?

from main:

...
	do
	{
		cout << "What was the score on your last test? "	<< endl	;
		cout << "Valid test scores are numbers between 0 and 100.\t" ;
		TestScore = getNumberInt()	;
	} while ( TestScore < 0 || TestScore > 100 )	;

...

int getNumberInt () //the heart of this is taken from the tutorial - ty C++.com
{
	using namespace std	;

	string UserResponse	;
	int UsableNumber	;

	getline(cin,UserResponse)	;
	stringstream(UserResponse) >> UsableNumber	;

        // now display what is actually in UsuableNumber just for fun...
	cout << endl << "Contents of UsableNumber:\t" << UsableNumber << endl	; // A hole is that the variable is not initialized, so, garbage in...
	// a workaround could be to initialize it with a value that will never be valid in any applications calling this object. kludge?

	return UsableNumber	;
}

I find from running the program that UsableNumber is an int (so far) always well outside the target range. Instictually this feels wrong to leave it to chance. It would be easy enough to initialize it to something outside the range required (which would trigger a reprompt from main) thereby avoiding any garbage - but it would be a kludge, which I read is frowned on.

While I'm still learning and will either find a solution or eventually learn enough to make my own my question is really about how frowned upon it is to kludge? and why? (beyond ego) ...

[firedraco]

If you are using the stringstream, you can simply check

if(stringstream(UserResponse) >> UsableNumber) {
    //worked
} else 
    //didn't
}

Then you won't have to worry really. In the case that you want to make your function more general (i.e., someone wants just an int, and all values could be valid) then you would throw an exception from the function if it didn't get an int.

Also, why are your ';' tabbed out? It looks really weird...

[helios]

I think that's a somewhat acceptable solution, but if you want to do it the right way, check (stream >> x). If it's false, the stream doesn't contain a human representation of a number and x wasn't changed.

[George10]

Thank you for your resposes... I have changed the portion in question based on them. From your responses I gather that a kludge is like a rookie mark - sure you could do it, but why would you when you could do without *and* have the program more functional.

A followup question on style then... more readable is better in the long run I have read. Both versions work, but version A seems more obvious about what is happening while version B is more compact and tricky, but is it too hard to read? would it be more acceptable with a comment and then 'better' than A?

VERSION A

int getNumberInt () 
{
	using namespace std;

	string UserResponse;
	const int Exit = 0;
	int UsableNumber, GoAgain=1;

	do
	{
		getline(cin,UserResponse);
		GoAgain = (stringstream(UserResponse) >> UsableNumber)? Exit : GoAgain;
	}while (GoAgain!=Exit);

	return UsableNumber;
}

VERSION B

int getNumberInt () 
{
        using namespace std;

	string UserResponse;
	int UsableNumber;

	do
	{
		getline(cin,UserResponse);
	}while (!(stringstream(UserResponse) >> UsableNumber));

	return UsableNumber;
}

I'm trying to get a sense of how to write good legible code.
Also, how much time (percentage wise) do you as a programmer spend 'de-kludging' what you write? I had something that worked ok on the first pass, but now it is certianly more robust and pretty. The trade off is that it certainly took more time.

thanks again!

[firedraco]

I'd go with B, since A basically contains an extra bool (as an int) for no real reason. Just check the condition directly.

Well, I would say it sort of depends...if it's something simple (like a member function that adds up some data in a class), then probably not much, but if it's a large function that does a lot of stuff, calls other functions, etc, then I might have to go over it once or twice to make sure I'm doing what I think I am. It also helps to have other people (programmers usually) look over your code and see if that can figure out anything that you might have screwed up on.

[helios]

Yeah, B. Definitely. I like terseness.

I try to avoid retouching code. If it works the way it needs to, I just leave it alone. I only retouch if the code doesn't quite work, or if I've come up with a much better solution. Otherwise I'd spend too much time testing and debugging the same pieces of code, rather than writing new code.

[herbert1910]

i do a bunch of needless variable assignment, mostly because i create tons of errors. the extra variables are extremely handy. if my mistakes were few, i would definitely pack the lines.

by the way, i consider VERSION A to be more difficult to understand because of that weird conditional/assignment thing. who does that outside of code obfuscation contests?

if you used GoAgain without the weird stuff, i would find VERSION A incredibly easy to understand compared to mentally parsing VERSION B's loop condition.

and doesn't GoAgain excite you? GoAgain! hell yeah! DontStop! oh yeah!

[helios]

The trinary operator is used everywhere. There's nothing out of the ordinary about line 12.

[herbert1910]

everywhere must be somewhere outside of the forums. these days, the trinary operator is rarer than a goto. we need a man like you to step up and save the endangered trinary operator and bring back the golden days when it appeared everywhere we looked.

by the way, is ternary also correct? that appears in the forums.

[helios]

They are synonyms. "Ter" comes from Latin and "tri" from Greek. The way I see it, if it's "binary", it should be "trinary", "tetrary", "pentary", "hexary", etc.

[z05DSL3A]

Technically the : ? operator call the conditional operator.
It is a ternary operation and is often known as the ternary operation.

In terms of the Arity of operators I have never heard Trinary used, I have only heard that used for number systems and it is interchangeable with ternary there.