Advertisement

How often do you revisit your hacky solutions or "ToFix" comments?

Started by October 17, 2013 01:10 PM
26 comments, last by Sirisian 11 years ago

Most programmers abhor unclean code and will naturally fix hacks if given the time (see 20% time).

But honestly, it depends on the hack. There are levels of hack from "ugh, slightly dodgy downcast here" to "mondo hackitude o rama" (thanks Microsoft biggrin.png ).

Didnt Bill Gates said he prefer lazy ppl because they find ways to do the work faster and easier?

Most programmers abhor unclean code and will naturally fix hacks if given the time (see 20% time).

But honestly, it depends on the hack. There are levels of hack from "ugh, slightly dodgy downcast here" to "mondo hackitude o rama" (thanks Microsoft biggrin.png ).

Didnt Bill Gates said he prefer lazy ppl because they find ways to do the work faster and easier?

Dunno if Gates said that, but I'd agree with it.

To paraphrase Terry Pratchett "it takes a lot of work to be truly lazy".

if you think programming is like sex, you probably haven't done much of either.-------------- - capn_midnight
Advertisement

I usually put down //TODO: and //POSSIBLE TODO: in my code. The former is generally something I will revisit as it's part of developing the functionality I've been assigned to do - I just need my brain to focus somewhere else for a moment. The //POSSIBLE TODO: category (that I might sometimes abbreviate to //TODO: stupidly enough) are things that at some undefined point in the future might become relevant to change. I rarely end up revisiting these.

Also have //HACK: and //NOTE: tags to warn for and explain WTF-code. Not using these nearly as often, for obvious reasons.

The only time I commit hacks is when we're up against a deadline, the fix is localized and the proper fix requires tons of testing to a bunch of systems. I generally have a proper fixed shelved in perforce for when the game is safe to commit these changes to again.

I often find myself fixing a lot of other engineers' TODO's. Often when tracking down bugs I'll debug down to the problem only to find a comment stating: "TODO: you need to do blah or it will break when blah happens"

I do sometimes leave TODO comments when I want to revisit an existing implementation later on when I have more time, but these aren't usually hacks, just not extensible in the way I'

When the number of "todo" comments crosses some arbitrary threshold - which varies in space-time, aka the "I gotta fix this mess" effect - I focus on getting them down to a sane level before continuing to add more features, if I have time (e.g. no imminent deadline or priority feature to implement). I eventually get around to it for projects I really care about. Eventually.

Well, it depends, of course. If it's things like "todo: improve this later on to support XXX" then I would probably implement it immediately after implementing said XXX out of necessity, unless I found a better alternative in the meantime. If it's a "todo: this could be better", that can wait (possibly indefinitely). If it's a "todo: [insert pseudocode]" then it's obviously just a placeholder to give high-level structure to my code until I actually get around to writing it. I try only to submit working and elegant code to the world, with as little todo's, missing bits and warnings (and free of as many bugs as possible) but sometimes nothing else will do in the time given, and you just gotta put that hack in to make the thing work like you want it to..

“If I understand the standard right it is legal and safe to do this but the resulting value could be anything.”

I wind up doing this with most pieces of code I write that require more than a little bit of thought. My first pass I code with the sole goal of getting whatever I'm trying to do working. Usually by the time it does work the eloquent, or at least not awful, solution presents itself.

Advertisement

it depends, does the "hack" save me several days worth of re-writing code? Then i can live with it. however, if i forsee re-doing things to be structured in a better format, and it would save me future work, i'll go back and redesign things if i need to.



void Serialize(Serializer &serializer, std::string &value)
{
	//Serialize the string size.
	uint16_t size = value.size();
	Serialize(serializer, size);
	value.resize(size);
	
	//Serialize the characters.
	char *cStr = const_cast<char*>(value.data()); //DANGER: Hack. I'm using the .data() function of std::string to write to std::string's internal buffer.
	serializer.Serialize(reinterpret_cast<Byte*>(cStr), size);
} 

Potentially very dangerous, but almost certainly workable in major implementations of the standard library - unless the implementation decides to copy its internal buffer at every call to .data(), or unless it has a duplicate buffer for some reason, .data() should return the internal char* buffer of the std::string.

That's clean code but hacky code. I have messier code in several locations, but I don't have anything hackier than that, iirc.

out of curiosity, why didn't you call c_str() instead?

Check out https://www.facebook.com/LiquidGames for some great games made by me on the Playstation Mobile market.

c_str is probably more likely to copy the data to put a null terminator on it? He's writing to the string not reading it. That code isn't going to null terminate the string which may violate an assumption the class makes (unless the null terminator is included in the size count). That code looks pretty dodgy to me.

If you read the whole data file into memory beforehand it's probably better to just construct the string from the raw bytes in the file buffer.

I'd just overload Serializer::Serialize to take a string argument though I think. Presumably it knows how to read bytes from the file so I'd read however many is needed then construct the string from the buffer.

"Most people think, great God will come from the sky, take away everything, and make everybody feel high" - Bob Marley

out of curiosity, why didn't you call c_str() instead?


.data() gets a char* string that is equivalent to the data the std::string holds. (pre-C++11)
.c_str() gets a null-terminated char* that is equivalent to the data the std::string holds.

This means that an implementation is more likely to create a copy of the string data for a call to c_str()... in theory. In practice, they almost certainly return the same string pointer.

In C++11, .data() is also required to be null-terminated. So they are even more almost-certainly going to be the same string pointer. Because of this, I chose the one the more closely documented my intent: I wanted to access the internal buffer (the 'data').

c_str is probably more likely to copy the data to put a null terminator on it? He's writing to the string not reading it. That code isn't going to null terminate the string which may violate an assumption the class makes (unless the null terminator is included in the size count).


.data() in C++11 is guaranteed to be null-terminated, just like c_str(). Since it's already null-terminated when I get the buffer (".........\0"), and I'm not writing over the terminator, the end result is still null-terminated..

That code looks pretty dodgy to me.

Absolutely. tongue.png That's what this thread is for, right? wink.png
It's pretty much the only true "hack" I have in my code - I have a few messy functions, but no other 'hacks'.

If you read the whole data file into memory beforehand it's probably better to just construct the string from the raw bytes in the file buffer.

I'd just overload Serializer::Serialize to take a string argument though I think. Presumably it knows how to read bytes from the file so I'd read however many is needed then construct the string from the buffer.

Yea, that would be ideal. I wish I could just hand std::string a null-terminated char* buffer for it to take ownership of, and that I could take() ownership of it's own null-terminated char* buffer (with the std::string then becoming an empty string, similar to move-semantics).

The problem with overloading Serializer::Serialize() for a string argument, is the std::string still won't take ownership of the char* buffer you'd read the data into - a completely unnecessary copy of every string read will always take place for no reason, unless you write directly to the string's buffer.

I should actually use &str[0] to get a non-const pointer to the internal data... oops. That'll let me avoid the const_cast, but it'll do the same thing anyway. Really, they ought to just make .data() return a non-const pointer. rolleyes.gif

I alway refactor/fix my shit unless its in code i havent visited in months (thats legacy code then lol)

This topic is closed to new replies.

Advertisement