• Advertisement
Sign in to follow this  

Are dirty coding techniques okay to use?

This topic is 2161 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

By "dirty", I mean brute forcing something to work just because you can't find an easy solution. Obviously the world isn't going to end if you brute force something, but I'm wondering if it would be best to just find a good solution, or if it's fine to brute force it to work. An example (although not a very good one) would be if you wanted to access a private member of a class that you knew the offset of within the class:


class Brute
{
private:
int _randomInt;
int _otherRandomInt;
char* _randomString;
};


Let's say that "_randomString" is the variable that you want to return, but perhaps you have no way of modifiying the "Brute" class to allow you access to this variable. Would it be a bad idea to create a function like this:


char *getBruteString(Brute *b_Ptr)
{
char *s_Ptr = (char*)b_Ptr;
//Get the address of _randomString by getting the address of the char at the 8th index in s_Ptr
char** c_Ptr = (char**)&s_Ptr[8];
return *c_Ptr;
}


I wouldn't write code like the above because it's really mess, but it should get the job done. Is it a bad idea to implement such types of methods if your code is only going to be used by your own team, and not released to public?

I really don't even want to post it because I feel like it seems like a silly question that undoubtedly has an obvious answer, I'm just curious of your opinion on the matter. If it works, it should be fine to use it, but I'm sure someone has a really good reason why it is a bad idea to use brute force coding.

Share this post


Link to post
Share on other sites
Advertisement
Is it a bad idea to implement such types of methods if your code is only going to be used by your own team, and not released to public?[/quote]

Is it bad to piss on your coworker's shoes as long as you don't do it in public?


char *getBruteString(Brute *b_Ptr)
{
char *s_Ptr = (char*)b_Ptr;
//Get the address of _randomString by getting the address of the char at the 8th index in s_Ptr
char** c_Ptr = (char**)&s_Ptr[8];
return *c_Ptr;
}
[/quote]
This particual example has a good and a bad side:
good: extension methods/free functions extend class while preserving encapsulation
bad: what it actually does

Pointer magic like that is so painfully compiler/build specific that if practices like this are allowed by team culture you'll end up with monolithic blob. it'll work, but one of two things will happen. Either some external factor will change, leaving you with potentially broken code or it will prevent you from advancing any part due to such obscure lock-in.

I don't consider techniques like these acceptable anymore. Not because of code - you'll find hackers who'll do crazier things. But because team culture that so casually allows them is throwing logs under their own feet.

Let's say that "_randomString" is the variable that you want to return, but perhaps you have no way of modifiying the "Brute" class to allow you access to this variable[/quote]

Why not? Maybe there's a reason? Can library owner provide such method? Why, why not?

Just because you can rob a bank doesn't mean it's first, second or even third choice when trying to pay for coffee.

Share this post


Link to post
Share on other sites
Occasionally, yes. But if you're going to take a short-cut, you should always carefully document what you've done and what needs to be done in order to fix things properly.

You're taking out a loan when you do stuff like that. Keep in mind that you'll eventually have to pay it back. With interest.

Share this post


Link to post
Share on other sites
I've been seeing all good replies in this thread. Like I said when I gave the above example, it's really not a good example. A better example would have taken longer to write.

I don't mean just limitations to retrieval of memory, I also mean for things like collision detection in a game. I was writing a 2D game engine that used tile maps, and I needed collision detection for the tiles. I COULD have done it the proper way (which would have taken a lot more code and problem solving), or I could have just brute forced it and done a lot of error checking. I would give some example code, but like I said, it would be too much to write.

Share this post


Link to post
Share on other sites
I've found that it is useful to distinguish between two kinds of coding activities: writing new code; and cleaning up / refactoring existing code.

When writing new code your focused to get the job done. Code tends to get messy. You take shortcuts. "All I want is a pointer to that string, hey a cast statement does the job, great". Keep in mind code structure and good practices but thinking too much about it and you'll risk slowing down progress and loose impetus.

When the code is in place and working, you can take a step back and look at it. Often you'll find patterns of repetition. "I'm casting this pointer almost every access, maybe I should change the type". Or violations of good practices. "Uh oh, I'm accessing that private member, better encapsulate it with an accessor method".

Brute force is ok for write-and-forget code. But if you expect to come back to the code or hand it over, better take some time and clean it up.

Share this post


Link to post
Share on other sites

Let's say that "_randomString" is the variable that you want to return, but perhaps you have no way of modifiying the "Brute" class to allow you access to this variable.

Even modifying 3rd party headers is preferable to the complete evil that is random address offsets. I don't care if Jesus smacked you with a bible-by-4 and said "thou shalt use this address instead of modifying mine headers" -- I'd still prefer it over this nonsense.

No code is perfect, and practicality takes precedence over trying to be perfect -- but even if you're going to do the wrong thing, don't use that as an excuse to not even try. Asserts, sanity checking, a few comments warning about how you're expecting it to break and why... try to make it so that, when it inevitably breaks, that's okay because the how and why are clear.

Random address offsets aren't going to break cleanly nor clearly. Down that path lies memory corruption, heisenbugs, and crashes 2 hours down the line in a completely unrelated part of your program.

Share this post


Link to post
Share on other sites

Occasionally, yes. But if you're going to take a short-cut, you should always carefully document what you've done and what needs to be done in order to fix things properly.

You're taking out a loan when you do stuff like that. Keep in mind that you'll eventually have to pay it back. With interest.


I completely agree with that. The occasion when stuff like that happens is usally before a deadline, because you need to get the stuff shipping. An the payback with interest is usually when code like that causes bugs which are not trackable or even reproducable because the code lacks all the control-mechanisms of "clean" code. And even if you can find and fix the bug, you still need to create a patch and get it out to your (possibly already very frustrated) users. The more frustrated your customers the more you have to rush the patch to keep em happy and before you know it, you're in the "fix one bug, introduce two new ones' loop.


Is it bad to piss on your coworker's shoes as long as you don't do it in public?


That is also another part of this situation. I and my team are responsible to maintain a framework for medical simulation and more often than not the one guy fixing some of the horrible code is not the guy that has written for it, which might lead to a lot of cursing about the original author. Also code like this often means that you are abusing a piece of software for a cause that it was not designed and thus not tested for or that you misinterpret a concept behind that library. Don't be afraid to ask the responsible developer "Hey, I want to do this, how do I do it without resorting to ugly hacking" even if you might feel stupid about it, because if you misunderstood a concept you will get guidance on how to adress your problem and if it is indeed a missing feature the developer will know about it and hopefully implement it in the future.

Share this post


Link to post
Share on other sites
A naïve algorithm is not necessarily "dirty" code.

The main pitfall is that if you have many such algorithms then you can end up with application with performance quicksand, under different circumstances it starts running very slowly when multiple "worst case" scenarios interact with each other, even though the application can handle any one such case in isolation.

On the other hand, simple implementations are sometimes easier to verify and maintain. Now, you were explaining that your dirty implementation has "a lot of error checking", which appears to say that the maintainability of this solution is questionable in this case.

There is no definitive right answer to the question, the best you can do is perform Cost / Benefit analysis. Even this is constrained to extent that the true costs and benefits can be accurately estimated in advance. A good engineer will gain, though experience, a better sense of intuition for when to choose a naïve algorithm over a theoretically superior one.

For instance, if the user interface only allows for 10 widgets to be displayed and manipulated, then having code that efficiently handles millions of widgets is less relevant than ensuring there are no bugs in the implementation.

Share this post


Link to post
Share on other sites

I wouldn't write code like the above because it's really mess, but it should get the job done. Is it a bad idea to implement such types of methods if your code is only going to be used by your own team, and not released to public?


To me, it's yes and no.
I would say NO if you write that code, then never back on it again.
But it's OK and Yes if you,
1, Are under heavy time pressure and want to finish your job quickly,
2, You are sure you will create a technical debt for yourself.
3, You are sure you will come back to solve that debt when you have time.
Then your code won't be that dirty because it won't exist permanently.

Share this post


Link to post
Share on other sites
dirty local code that is encapsulated is okay-ish for quickly getting something to work. but fix it afterwards.

dirty global code (globals, singletons, too-big-to-fail classes) should be avoided at all cost. especially, as they typically never help in getting something done quickly, anyways. but cleaning them up afterwards can be hell.

Share this post


Link to post
Share on other sites
Hacks like your first post are not ok. If that really is the only way to do what you want to do, use another library as it seems broken by design. Mostly though, there has to be another way to do it.

Implementing sub optimal algorithms on the other hand can be ok if that is not your main concern at the moment. When I am in that situation I usually add a comment saying: //HACK //TODO //SLOW or something like that and log a warning message. So if later I wonder why my code is performing poorly, I know where to look.

Share this post


Link to post
Share on other sites

Implementing sub optimal algorithms on the other hand can be ok if that is not your main concern at the moment. When I am in that situation I usually add a comment saying: //HACK //TODO //SLOW or something like that and log a warning message. So if later I wonder why my code is performing poorly, I know where to look.



One of the code bases I am working on now was started in 1997, and it has literally thousands of those (plus a few hundred special ones that mean "we are *really really going to do this right when we have time, I swear). laugh.png

The thing about those, is that they tend to never go away once they are made. No matter how strong the intentions were at first to really change it when there is time, just a few weeks or even days later everyone has forgotten about them and moved on.

I think it's OK to make hacks sometimes, if you are convinced that they will work well enough, and not suddenly stop working. The example in the OP is a hack that is subject to suddenly stop working, and thus never OK.

Most of the ToDos and FixMe:s in code seem to be there because the original programmer knew of a better way, but for some reason didn't do it, so the comment was placed there to ease his own "guilt".

Share this post


Link to post
Share on other sites
The big problem with hacks like this, and the reason why they tend to become permanent "features" of a codebase, is that they can cause subtle side-effects that may then subsequently come to be relied on for other required behaviour. So instead of sitting down to write the code properly you decide that it's OK because it works anyway, but you risk missing the point that it's not working by design; it's working by accident.

When the time does come to clean it up, or to add new features to it, you may have accumulated a large amount of other things in the project that have a reliance on the initial misfeature, meaning that you can't do so without risk of breaking these other essential features.

However, I'm not even going to pretend that this is a prescriptive answer. Sometimes you do have to do things like this. It sucks, you'll feel dirty, but it's the only way. But do make sure that it's a last-resort option, do make sure that you've exhausted all other possibilities, and do make sure that you've read the relevant documentation/consulted the relevant authorities before brute-forcing it.

Share this post


Link to post
Share on other sites

Most of the ToDos and FixMe:s in code seem to be there because the original programmer knew of a better way, but for some reason didn't do it, so the comment was placed there to ease his own "guilt".


I think the reason is usually time constraints, quite often a feature is needed yesterday (Either for a customer or for the rest of the team), quick solutions gets added, a comment written and as long as it works and doesn't cause any problems it will remain.

I think the important thing here is to not just add a comment to the code but to use a format for those comments that your documentation tools can parse, (That way you can get a good overview of all the quick hacks that exist in your codebase and the potential negative effects they have. (and if you don't use documentation tools it is a good idea to write a separate index for the things that should be fixed if there is ever time for it).

Personally what i want to know, just by looking at the documentation is:
The location of the hack, (File, namespace, class, function, etc)
The reason for the "hack" being used. (Time constraints ? , easier to reason about ?)
The potential drawbacks of the hack, (poor performance ? , bad scaling ? security concerns ? portability ? tight coupling ?)
Estimated importance of replacing the hack with a better solution. (I probably won't have time to fix them all, if i do have time i'd like to know where to start) and if possible a list of the better solutions that weren't used due to the reasons listed above.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement