Coding Horrors: You can gather all your technical debt into one easy framework!

The Something Awful Forums > Discussion > Serious Hardware/Software Crap > The Cavern of COBOL > Coding Horrors: You can gather all your technical debt into one easy framework!

«‹›2 »

TSDK: Nov 24, 2003; I got a wooden uploading this one

zootm posted:

I think either some spacing or avoiding the ternary operation altogether would make the "after" code in that example a lot easier to read, JingleBells.

It would also help in terms of performance, because then you're not doing two lookups:

code:

public string getProperty(string key) 
{
    map::iterator property = map[key]; // (I don't know the synax)
    return ( property == null ? null : property.ToString() );
}

More readable code == faster code. Who'd have thunk it?

# ¿ Apr 10, 2008 14:32

Adbot: ADBOT LOVES YOU

# ¿ Apr 24, 2024 11:11

TSDK: Nov 24, 2003; I got a wooden uploading this one

MasterSlowPoke posted:

the language is stackless python, definitions cut off for brevity
code:
/********* DO DFT *******/

Code for DFTs and suchlike almost always looks like that. It's the same with a lot of academic code too. Because the paper they're working from has got the formulae written out with single letter, subscripted variables then you often find just a straight transcript from paper to code.

It doesn't usually matter too much on self contained mathematical operations like that, because you can prove the correctness of the routine with relatively straightforward regression testing and then leave it alone for years.

The most important thing is: does it have the reference to the original paper in the comments?

# ¿ Apr 15, 2008 09:26

TSDK: Nov 24, 2003; I got a wooden uploading this one

JoeNotCharles posted:

No, because then you need a whole bit for each individual choice. Once you get past 32 choices it takes more than a word of memory, so it's very space-inefficient. With primes, you can fit 4294967296 different options in a word (well, not that many because not every number is prime, but it's still a hell of a lot more than 32).

I would say that you can only encode 9 flags using primes before you overflow a 32 bit unsigned word, but I just don't know who's joking or not any more

# ¿ Apr 24, 2008 18:04

TSDK: Nov 24, 2003; I got a wooden uploading this one

Aredna posted:

You could let it overflow and just assume you're doing the math mod 2^32. I haven't looked, but I'd imagine you can go quite a ways before you ran into a collision.

Oh god, you're right. I'm in two minds whether just to recoil in horror, or to download a primes list and put it to the test.

:psyduck:

Anyone fancy putting the appropriate loading code in and testing?:

code:

#include <list>
#include <set>
#include < (whatever it is for std::cout, I can't be bothered to look) >

typedef std::list<unsigned int> tPrimeList;
typedef std::set<unsigned int> tTotals;

int main()
{
    tPrimeList primes;
    // Read primes.txt into primes here

    tTotals used;
    unsigned int flag_count = 0;
    unsigned int running_total = 1;
    for ( tPrimeList::iterator prime = primes.begin(),
                               end = primes.end();
          prime != end;
          ++prime )
    {
        running_total *= *prime;
        if ( primes.find( running_total ) != primes.end() ) break;
        primes.insert( running_total );
        flag_count++;
    }

    std::cout <<
        "Can encode " <<
        flag_count <<
        " flags before a collision occurs" <<
        std::endl;
}

# ¿ Apr 24, 2008 19:27

TSDK: Nov 24, 2003; I got a wooden uploading this one

Aredna posted:

The real question is, what algorithm do you use to decode the number back to base primes? What if you don't know how many or which primes were used other than that they produced no collisions?

You cache it all in a std::map< unsigned int, std::set< unsigned int > >, of course, duh!

# ¿ Apr 24, 2008 21:17

TSDK: Nov 24, 2003; I got a wooden uploading this one

minato posted:

Unless you compress the result

What if, now hear me out guys, what if we use this compression scheme where we represent each prime by a particular power of two. So the i-th prime is encoded as 2^i, and then add these together to represent which primes are factors of the number we're compressing.

By my rough calculations, we should be able to store all of the values we're after up to 31610054640417607788145206291543662493274686990, in a little under 33 bits.

Then when we want to figure out which values are set, we just multiply up the primes and then factor the number as usual. Job done.

minato posted:

We're through the looking glass here folks

And how.

# ¿ Apr 25, 2008 09:24

TSDK: Nov 24, 2003; I got a wooden uploading this one

floWenoL posted:

A simple dataflow analysis pass should enable the compiler to conclude that the copied variable is not used and therefore can be omitted, assuming the copy constructor etc. have no side effects.

(Emphasis mine)

That's the biggie though, and I'm not sure that's common to a lot of compilers.

I'm willing to bet that the same code was generated on each for the vector example because the iterator is a simple typedef to a pointer. What happens with a std::list or std::map?

# ¿ May 7, 2008 12:19

TSDK: Nov 24, 2003; I got a wooden uploading this one

tef posted:

They wanted to get rid of warning messages caused by valgrind.

Not sure if I'm reading the thread right, but are they really relying on uninitialised variables as a source of random data? That's a serious WTF right there, because there's absolutely nothing to say that uninitialised variables should contain non-predictable data.

tef posted:

At this point I think this says it best:

Linus Torvalds posted:
Please, we do NOT fix compiler warnings without understanding the code! That's a sure way to just introduce _new_ bugs, rather than fix old ones. So please, please, please, realize that the compiler is _stupid_, and fixing warnings without understanding the code is bad.

And that's a second WTF right there. Compilers are not stupid. Only people who think compilers are stupid, are stupid.

EDIT: Yes, I know I've just called Linus Torvalds stupid, but before the usual "Omg, but Linux, waaaaah!" starts up: I don't give a drat what he's done in the past, if he's going to say something stupid then I'll call it stupid.

TSDK fucked around with this message at 15:56 on May 13, 2008

# ¿ May 13, 2008 15:45

TSDK: Nov 24, 2003; I got a wooden uploading this one

Smackbilly posted:

Well, the admonition not to fix warnings without understanding the code is valid - in fact you shouldn't do anything to code that you don't understand.

The admonishment may be correct for code that loads of other people are using and relying on is valid, but the reasoning is not.

I also disagree that you need to have a full and complete understanding of anything before coding or bug fixing. Often the only way to get to fully understand systems is when you actually go through it in practice trying to change or implement something. It's also perfectly possible to do an adequate job fixing bugs without understanding all of the surrounding code.

Smackbilly posted:

For example, consider
code:
if (x = y) {
 // stuff
}
Many compilers will emit a warning for this, because they can't possibly know if you made a typo, or really wanted to assign y to x and then test the value of x for truth. There's a way to get around this of course (extra parens) but that's an extra bit of otherwise meaningless code that exists only to placate the compiler.

It's not just the compiler you need to convince; if I saw that in code then I would go through the logic anyway to just to check that the coder hasn't made a genuine mistake. I imagine that most experienced coders would do likewise. The fully correct way to fix it is to put in the double parens, something you're unlikely to do by accident if it was a mistake, and ideally plonk a comment saying that you're doing it intentionally.

# ¿ May 13, 2008 16:48

TSDK: Nov 24, 2003; I got a wooden uploading this one

Mikey-San posted:

I think Torvalds means "stupid" in the sense that "it only knows a set of predefined rules". It cannot determine the intent of the programmer, so while it's good at what it does, it can't think. It's "dumb".

Yeah?

Whilst that is a reasonable statement about compilers, I'm still worried that someone's default attitude to warnings would be "I'm a programmer, I know best". Compiler warnings and sanity checking tools are there to help - as soon as false positives start creeping in unchecked, then they become useless. There's nothing worse than finding a bug the compiler was warning you about, but you didn't see it because it was hidden amongst a mass of 'harmless' warnings.

# ¿ May 13, 2008 17:17

TSDK: Nov 24, 2003; I got a wooden uploading this one

tef posted:

It turns out I'm talking poo poo!
http://www.links.org/?p=327

It seems to me then that there were 3 WTFs:

1) Relying on specific, non-guaranteed behaviour from uninitialised memory.
2) Knowing that you get specific warnings about sections of code and failing to comment them appropriately.
3) Making a change to some code you don't fully understand without getting in touch with either the original author, or someone who could explain why it's doing what it is.

If you ask me, then the original OpenSSL devs are just as much to blame for that clusterfuck as the guy who put in an incorrect 'fix' for the problem.

# ¿ May 14, 2008 10:13

TSDK: Nov 24, 2003; I got a wooden uploading this one

EssOEss posted:

== is generally defined as reference equality, not data equality. Overriding it to perform data comparison would be a Coding Horror.

No it wouldn't.

Begone, foul Java demon!

# ¿ May 16, 2008 09:08

TSDK: Nov 24, 2003; I got a wooden uploading this one

Jo posted:

Is there a reason someone would write
code:
if( ((num-1) & (num)) == 0 )
...rather than...
code:
if( num%2 == 0 )
Or am I being dense?

The first test is checking to see if a number is a power of two:
http://aggregate.org/MAGIC/#Is%20Power%20of%202

The second one is testing for an even number, so the tests are not the same.

# ¿ Jun 3, 2008 15:33

TSDK: Nov 24, 2003; I got a wooden uploading this one

In any case, the correct form of the test is:

code:

if ( IsPowerOfTwo( num ) )
{
    ...

That way there's no questions about what the test is actually doing, and you only ever need to look at the (num)&(num-1) shenenigans if you're in the middle of reading a function called IsPowerOfTwo - by which time it's obvious what the bit twiddling is for.

# ¿ Jun 4, 2008 09:56

TSDK: Nov 24, 2003; I got a wooden uploading this one

CeciPipePasPipe posted:

Uh, wouldn't *(int*)13 = 3 cause a general protection fault (and possibly a bus error on archs that require aligned memory access), because you're trying to write the value 0x00000003 into memory at location 0x000000D ? I don't see how this has anything to do with interrupt 3.

Interrupt 3 is a break to debugger. Causing a general protection fault will also stop execution and drop you to the debugger in most cases.

The only disadvantage is that you normally can't hit F5 and continue execution after hitting the latter, as you can with the former.

Personally, I use a 'DEBUG_BREAK()' macro which is defined on a per-platform basis.

# ¿ Jul 9, 2008 12:30

TSDK: Nov 24, 2003; I got a wooden uploading this one

CeciPipePasPipe posted:

Oh ok, wouldn't it be easier to just do "int breakme=1/0;" or something then?

No, because div by zero exceptions aren't necessarily enabled or available.

# ¿ Jul 9, 2008 14:16

TSDK: Nov 24, 2003; I got a wooden uploading this one

Entheogen posted:

why couldn't one do this
code:
#define DEBUG_BREAK exit(1)

Because we're talking about coding horrors, not trying to create more of them?

# ¿ Jul 9, 2008 15:05

TSDK: Nov 24, 2003; I got a wooden uploading this one

Cross_ posted:

Ignoring the fact that this betrays bad design- you have to change the type declaration anyway what's the problem with doing search&replace at the same time?

It fucks over your source control change logs, and is a very blunt weapon considering that not all matching strings in the source code are necessarily matching variables.

Hungarian notation is an old technique that has no place in modern C++ programming, but there are one or two odd exceptions which are either genuinely useful, or are a strange hang-over.

The scope of variables is of critical importance in C++, and in OO programming in general. As such, you often see 'g_', 's_' and 'm_' prefixes used widely - or similar conventions for distinguishing member variables.

The 'p_' prefix is more of a comfort thing. There's not necessarily any reason why it should be used, but it's a fairly firmly entrenched an widely used habit. It generally doesn't cause as many problems as other types of type prefix though. One of the main arguments against Hungarian is that if you change the type, then you have to hunt out and change everywhere the variable is used for no good reason. Whereas if you change a variable to or from a pointer type, then you will generally have to change all of the places where it's being used anyway, so the negative impact of this particular prefix is relatively light.

I've not seen 'r' being used all that often, and I suspect it's a dirty Java habit

# ¿ Oct 2, 2008 23:41

TSDK: Nov 24, 2003; I got a wooden uploading this one

My favourite coding horror side effect of Hungarian notation is little gems like this scattered around here and there:

code:

float uiItemSize = pItem->GetSize();
...

# ¿ Oct 3, 2008 11:29

TSDK: Nov 24, 2003; I got a wooden uploading this one

Fib posted:

http://www.urubatan.info/2008/11/commenting-source-code-is-only-for-the-weak/

If he thinks that this:

code:

closeFileReader(readerForFile);

Is somehow more readable than:

code:

readerForFile.close();

Then perhaps he should be programming in C and not Java.

# ¿ Nov 20, 2008 18:08

TSDK: Nov 24, 2003; I got a wooden uploading this one

... and why is a function for reading called 'write' at one point?

# ¿ Nov 20, 2008 19:14

TSDK: Nov 24, 2003; I got a wooden uploading this one

tripwire posted:

Ulillillia released the source code for a program he designed in C which converts the sampling rate of PCM wave files.
Check it out:
http://www.ulillillia.us/files/WAVFileSampleRateGeneratorV2Source.zip

Oh my.

Oh my-o-my.

Is that someone on the forums?

# ¿ Dec 16, 2008 14:30

TSDK: Nov 24, 2003; I got a wooden uploading this one

hexadecimal posted:

There will never be any other objects other than CVSRevision coming in there. If an object of other types comes through, this will be an error in my program, and will be caught in another place,

Why not throw in an exception or an assert, just to be sure?

Then when the thing that should never happen actually does happen, you'll have a head start on tracking it down and fixing the bug.

# ¿ Jan 8, 2009 18:31

TSDK: Nov 24, 2003; I got a wooden uploading this one

Presto posted:

Strictly speaking, this:
code:
union {
    int i;
    float f;
} butts;

float x;

butts.i = 666;
x = butts.f;
is not guaranteed to work either. You can't assign to one union member and then read another union member of a different type. The exception is that if the union members are structures with the same initial sequence of members and the union currently holds one of those structures.

Actually, union casting is guaranteed to 'work' in the presence of strict aliasing (spit), it's just that the values returned by reinterpretting the bit pattern from one type to another aren't defined.

# ¿ Jan 8, 2009 20:47

TSDK: Nov 24, 2003; I got a wooden uploading this one

I've started seeing this rather sub-optimal anti-pattern in a few places recently:

code:

if ( stricmp( "SOME_STRING_1", p_string ) == 0 )
{
    // Do stuff
}
if ( stricmp( "SOME_STRING_2", p_string ) == 0 )
{
    // Do stuff
}
if ( stricmp( "SOME_STRING_3", p_string ) == 0 )
{
    // Do stuff
}
if ( stricmp( "SOME_STRING_4", p_string ) == 0 )
{
    // Do stuff
}
etc...

Because, y'know, we've got nothing better to do with that CPU time other than carry on testing strings after we've already had a positive match.

# ¿ Jan 27, 2009 21:23

TSDK: Nov 24, 2003; I got a wooden uploading this one

Otto Skorzeny posted:

How many strings are you testing against by the way

Enough to be annoyed at the repeated tests, but not enough to warrant a data structure to map the cases better.

# ¿ Jan 28, 2009 10:24

TSDK: Nov 24, 2003; I got a wooden uploading this one

Mill Town posted:

Data structure hell, just stuff all the strings into a hash set.

Near-constant lookup time, it'll actually perform better than an if-else block.

1) There is no standard library hash set in C++ (and this is an embedded system, so boost is not an option without butchering up a custom version).
2) They're stricmps, not strcmps, so you'd still need to do some faffing to get the hashing (or sorting if you want to use a std::map) working properly.

It's only done once per load, so it's not in a performance critical section, but it is annoying that it'll take longer than it should for want of a few extra keywords. It's sloppy rather than outright terrible.

# ¿ Jan 28, 2009 10:57

TSDK: Nov 24, 2003; I got a wooden uploading this one

ehnus posted:

A coworker stumbled across this and forwarded it to me:

At first I thought it would be a really retarded way of getting round a:

code:

warning C4800: 'BOOL' : forcing value to bool 'true' or 'false' (performance warning)

warning message. But the first line in the function causes that warning to be spat out anyway.

# ¿ Feb 12, 2009 12:44

TSDK: Nov 24, 2003; I got a wooden uploading this one

Trammel posted:

The 'not-invented-here' syndrome strikes again!

A friend of mine has been asked to replace the calls to functions in System.Security.Cryptography in a C# app he's working on with calls to encryption functions written in-house in order to 'increase' security. Needless to say that the in-house encryption functions amount to nothing more than a polyalphabetic cipher with a fixed key.

Mmm, I love me some security.

# ¿ Feb 12, 2009 21:49

TSDK: Nov 24, 2003; I got a wooden uploading this one

quote != edit

# ¿ Feb 12, 2009 21:50

TSDK: Nov 24, 2003; I got a wooden uploading this one

GrumpyDoctor posted:

homegrown resizable array that's basically std::vector without bounds checking

Just so we're clear, unless you're using .at(n), then std::vector doesn't have to have bounds checking either. Some implementations choose to add bounds checking to operator[] in debug builds, but it'll get compiled out in a release build.

I know you probably know that, but it is a common myth about the STL.

# ¿ Feb 13, 2009 10:35

TSDK: Nov 24, 2003; I got a wooden uploading this one

Richard Noggin posted:

I can certainly understand using descriptive variable names, but you have to be a complete loving moron to require them in loops.
code:
for (int index = 0; index <= carpalTunnel; index++)
{
    for (int index2 = 0; index2 <= rsi; index2++)
    {
        //you can see where this can get confusing
    }
}
Maybe not a true coding horror, but a pain in the rear end at the very least.

The reason why that is a horror is because 'index' is no more descriptive than 'i'.

Now if you'd have put:

code:

for ( int mesh = 0; mesh < num_meshes; mesh++ )
{
    Mesh &current_mesh = meshes[mesh];
    for ( int triangle = 0; triangle < current_mesh.NumTriangles(); triangle++ )
    {
        // Do stuff
    }
}

Then you can clearly see the extra meaning that you get from using longer variable names. If you're just going to use 'index', then you might as well be allowed to use 'i'.

# ¿ Feb 24, 2009 17:57

TSDK: Nov 24, 2003; I got a wooden uploading this one

Zombywuf posted:

What's you're problem,

Ahem...

code:

class LogSingleton {
public:
	static void PrintMessage (std::string text) {
		std::cout << text << "\n";
	}
	
	static void PrintError (std::string text) {
		std::cout << "AN ERROR" << text << "\n";
	}
};

# ¿ Mar 13, 2009 12:18

TSDK: Nov 24, 2003; I got a wooden uploading this one

Zombywuf posted:

I'm assuming the original developer was planning to add more sophisticated code later, like locking, but didn't get the time.

Basically it's overcomplicated for what it does right now, but it's pretty clear as to how it's doing what it's doing.

Perhaps he/she didn't get the time because he/she was always adding in code that wasn't actually needed.

Coding for the future can be appropriate, however, there's also a lot to be said for waiting until you have a better idea of what's needed before writing code. That way if you never need it then you've not wasted any time, and if what you actually turn out to need is different to what you thought you'd need, then there's less code that needs changing.

# ¿ Mar 13, 2009 14:25

TSDK: Nov 24, 2003; I got a wooden uploading this one

Mick posted:

code:

Your avatar is perfect to go with that post.

# ¿ Mar 26, 2009 22:39

TSDK: Nov 24, 2003; I got a wooden uploading this one

Wheany posted:

But also:
The flag is defined and manipulated, but never tested, in device.c.
It is declared extern and tested, but never manipulated, in driver.c.

Why would you think this is a coding horror? Sure, it's a bit of an old-school way to get a status from a module, but then you are working in C.

# ¿ Apr 8, 2009 11:13

TSDK: Nov 24, 2003; I got a wooden uploading this one

I came across a good one-liner today:

code:

sbp->p_buffer = p_destination ? p_destination : 0;

# ¿ Jun 26, 2009 16:47

TSDK: Nov 24, 2003; I got a wooden uploading this one

quadreb posted:

Hey concurrency, how's it going?

I'm doWhat'ing fine,s con thankcurreyou for asking.ncy?

# ¿ Jun 30, 2009 18:16

TSDK: Nov 24, 2003; I got a wooden uploading this one

Vinterstum posted:

Something dumb like wanting some of their internal data types to be 16 byte aligned, which the VS STL implementation can't handle (We actually had problems with this at work as well, I remember).

Oh god, I remember that problem too. The cause (back on VC++ 6 & 7.1) was that you can't pass aligned objects by value as function arguments. I think our solution was just to patch a couple of the functions within the Dinkumware implementation to take those arguments as const references, and all was well.

Vinterstum posted:

Again: Oh, the horror!

One thing not mentioned, which is often quite important within the games industry, is serialisation. If you have your own container implementation, then you can guarantee the layout and load it in directly as a binary image. Of course, there's nothing to stop you doing that and simply giving your own array container (more or less) exactly the same interface as a std::vector anyway.

# ¿ Jul 1, 2009 10:31

Adbot: ADBOT LOVES YOU

# ¿ Apr 24, 2024 11:11

TSDK: Nov 24, 2003; I got a wooden uploading this one

tef posted:

This would explain the response I got to this childish job advert.

code:

Job Advert                  ## FAIL
Company                     ## FAIL
Potential Customers         ## Shun them. FAIL
Their Bank Manager          ## Hates them... FAIL
5+ Ninjas at dole office    ## FAIL NINJA-FAIL

# ¿ Jul 9, 2009 12:14

The Something Awful Forums > Discussion > Serious Hardware/Software Crap > The Cavern of COBOL > Coding Horrors: You can gather all your technical debt into one easy framework!

«‹›2 »