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

Vinterstum · # ? Jun 30, 2009 20:22

TheSleeper posted:

How many items in that "Motivation for EASTL" have to do with portability/compatibility? One. How many have to do with performance and/or common issues that come up in game programming but not elsewhere? (hint: it's all but the one).

The Bullet library is primarily used in the games domain, so what does it matter what's true elsewhere?

I'm not saying the STL is bad, but there's certainly some valid arguments against using it in console development. Which is why calling the decision a "coding horror" is just absurd.

Adbot · # ? Apr 26, 2024 20:15

Zombywuf · Zombywuf Mar 29, 2008

Vinterstum posted:

I'm not saying the STL is bad, but there's certainly some valid arguments against using it in console development. Which is why calling the decision a "coding horror" is just absurd.

Nothing wrong with writing their own STL implementation for consoles, apart from STL implementations being a dime a dozen. Or with writing their own STL for performance reasons (the EASTL doc reeks of crazy though). Instead they decided to use their own array object for compatibility reasons?

Really the EASTL doc reads like it was written by a genius, about 20 years after his peek, just as he's starting to be destroyed by drink.

Vinterstum · # ? Jun 30, 2009 22:13

Zombywuf posted:

Nothing wrong with writing their own STL implementation for consoles, apart from STL implementations being a dime a dozen. Or with writing their own STL for performance reasons (the EASTL doc reeks of crazy though). Instead they decided to use their own array object for compatibility reasons?

Really the EASTL doc reads like it was written by a genius, about 20 years after his peek, just as he's starting to be destroyed by drink.

Looks like it was for Visual Studio compatibility: http://bulletphysics.com/Bullet/phpBB3/viewtopic.php?f=9&t=3638&p=13749&hilit=std%3A%3Avector#p13749

Oh the HORROR!

geetee · # ? Jun 30, 2009 22:15

Senior developer wrote this:

mysql> EXPLAIN SELECT terminal_inventory_history.* from terminal_inventory_history INNER JOIN period ON terminal_inventory_history.settlement_period_id = period.id WHERE terminal_id IN (1183, 1378, 1393, 2440, 3575, 17695, 17709, 17722, 17738, 17827, 17852, 17889, 18001, 18035, 362170, 18192, 18229, 339411, 18445, 18471, 18860, 18872, 186617, 18985, 19025, 19052, 339588, 339600, 19264, 339626, 19432, 19452, 19472, 19486, 19715, 19755, 19801, 19810, 19828, 19855, 19980, 20101, 20208, 20241, 20388, 20433, 20484, 20546, 20584, 20644, 20680, 20737, 20784, 20815, 339942, 20932, 20950, 21184, 21273, 21484, 21503, 21651, 21701, 22044, 340240, 22201, 25230, 25251, 25382, 25392, 25406, 341268, 78774, 79483, 79570, 79596, 187424, 9059106, 19317, 9059520, 21369, 21402, 21356, 21351, 9060320, 22056, 22142, 9061375, 342812, 22559, 22692, 32079, 22764, 22740, 9539, 23415, 23635, 23623, 23843, 24258, 337119, 340971, 25170, 24357, 24470, 24901, 9080631, 79824, 24754, 24894, 341208, 80063, 9083754, 80343, 80497, 337590, 80504, 80489, 357879, 80565, 9088890, 9088897, 9089963, 80713, 80846, 357972, 81389, 100606, 82081, 81746, 81704, 81871, 14711, 14697, 249745, 82533, 82279, 9100835, 82670, 82709, 82812, 101082, 358345, 16521, 82979, 83154, 83196, 358441, 108530, 83529, 83764, 83716, 358549, 83858, 83884, 84043, 84197, 84046, 84303, 84351, 84692, 111312, 251844, 84806, 111555, 84941, 85054, 358860, 85340, 112727, 85517, 102930, 363255, 252556, 85662, 85681, 85736, 85874, 86011, 113959, 86096, 114499, 86339, 86367, 114991, 86433, 9120097, 115407, 86634, 116126, 86875, 116235, 87129, 89271, 359459, 87337, 87620, 359728, 119794, 119856, 286278, 88238, 88403, 88533, 89062, 89687, 89753, 124959, 124943, 90171, 90182, 126046, 90979, 90957, 292859, 91196, 91251, 128730, 91267, 128826, 128921, 91331, 91470, 91504, 129879, 91599, 91673, 258511, 130654, 91889, 131247, 91978, 131320, 361020, 372038, 106335, 92361, 92481, 106634, 106875, 92733, 134503, 134575, 92823, 92879, 373108, 93044, 9162176, 361526, 93709, 93773, 94078, 94090, 107063, 94470, 94515, 139524, 139764, 140179, 94813, 374835, 447784, 4160694, 140895, 308272, 107233, 141519, 107745, 142788, 144679, 142960, 95839, 95869, 107679, 144083, 144449, 96315, 96290, 96521, 96510, 146520, 145683, 411492, 146509, 97246, 97429, 148531, 412669, 400471, 98445, 98451, 151621, 151947, 99016, 99025, 99014, 320210, 948780, 153485, 99645, 949010, 949053, 961646, 9198620, 961981, 949090, 949167, 962228, 9199480, 962324) ORDER BY terminal_inventory_history.activation_time DESC, period.end_time DESC\G

*************************** 1. row ***************************
id: 1
select_type: SIMPLE
table: terminal_inventory_history
type: range
possible_keys: terminal_inventory_history_terminal_idx
key: terminal_inventory_history_terminal_idx
key_len: 8
ref: NULL
rows: 146183
Extra: Using where; Using temporary; Using filesort
*************************** 2. row ***************************
id: 1
select_type: SIMPLE
table: period
type: eq_ref
possible_keys: PRIMARY
key: PRIMARY
key_len: 8
ref: dbname.terminal_inventory_history.settlement_period_id
rows: 1
Extra:
2 rows in set (0.01 sec)

Triple Tech · # ? Jun 30, 2009 22:31

I can't really think of a good way to select multiple objects by ID in SQL. Granted, it looks a little funny because there are lots of IDs in that particular query... But really, what else are you supposed to do? I figure, maybe insert the IDs into an indexed temporary table of one column and then run a join against that? How much faster could that be.

geetee · # ? Jun 30, 2009 22:48

I should have mentioned that the list of IDs is pulled out by something like:
SELECT `terminal_id` FROM `table` WHERE `user_id` = 123

Basically, JOIN 101 poo poo. To make matters worse, he didn't see the problem with the EXPLAIN.

Zakalwe · # ? Jul 1, 2009 00:16

Vinterstum posted:

Looks like it was for Visual Studio compatibility: http://bulletphysics.com/Bullet/phpBB3/viewtopic.php?f=9&t=3638&p=13749&hilit=std%3A%3Avector#p13749

Oh the HORROR!

I have a really hard time believing that VS2008 has a broken implementation of something as pervasive in C++ code as std::vector.

Instead I'm inclined to believe they're doing something dumb and out of spec that works in GCC and therefore "VS2008 is broken".

Bullet's Demos are also the most convoluted pieces of code. Whole bunches of similar classes are constructed via Macros. Each demo inherits from a huge Demo class. Each demo shares random files with other demos. Heaven forbid examples and demos are easy to understand.

Another WTF: The multithreaded implementation was first realised on the Cell. The more generic Linux/Windows code is a *direct* port of this. Each worker thread operates with a 256k local store that it has to populate via memcopy() from the main pool :psyduck:

ehnus · # ? Jul 1, 2009 01:23

Zombywuf posted:

Nothing wrong with writing their own STL implementation for consoles, apart from STL implementations being a dime a dozen. Or with writing their own STL for performance reasons (the EASTL doc reeks of crazy though). Instead they decided to use their own array object for compatibility reasons?

Really the EASTL doc reads like it was written by a genius, about 20 years after his peek, just as he's starting to be destroyed by drink.

Paul is a genius, and you couldn't be more wrong on everything else. There isn't any "crazy" in EASTL beyond what has been required and requested by internal game development teams.

haveblue · # ? Jul 1, 2009 02:27

Are we really having an argument about code quality in the game industry? That's like discussing the environmental impact of Formula 1 cars.

ChiralCondensate · # ? Jul 1, 2009 02:45

Just to lighten things up a bit:

code:

friend class kkkwwwxxxyyyzzz; //just to avoid the compiler warning...

Mikey-San · # ? Jul 1, 2009 02:59

sex offendin Link posted:

Are we really having an argument about code quality in the game industry? That's like discussing the environmental impact of Formula 1 cars.

code:

// drunk, fix later

Vinterstum · # ? Jul 1, 2009 07:18

Zakalwe posted:

I have a really hard time believing that VS2008 has a broken implementation of something as pervasive in C++ code as std::vector.

Instead I'm inclined to believe they're doing something dumb and out of spec that works in GCC and therefore "VS2008 is broken".

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).

Again: Oh, the horror!

Vinterstum fucked around with this message at 07:20 on Jul 1, 2009

TSDK · # ? Jul 1, 2009 10:31

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.

Zombywuf · # ? Jul 1, 2009 13:08

ehnus posted:

Paul is a genius, and you couldn't be more wrong on everything else. There isn't any "crazy" in EASTL beyond what has been required and requested by internal game development teams.

Internal niche market software being well known for it's high quality.

Seriously I was expecting some super cool cache friendly template specialisations, and I got a ranty bizare document with an altogether too heavy focus on linked lists.

The Red Baron · # ? Jul 2, 2009 00:58

PHP 5.3: now with goto

chips · # ? Jul 2, 2009 01:16

Who are Dmitry and Sara why are they allowed near PHP?

edit: Ok I do sometimes use goto in C++

Zakalwe · # ? Jul 2, 2009 01:37

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).

Again: Oh, the horror!

They claimed VS was buggy. This is not a bug and therefore std::vector in Visual Studio's STL implementation is not broken in this regard. Perhaps the real WTF is the cheap LOL MS shot for no other reason than VS is not GCC.

baquerd · # ? Jul 2, 2009 01:44

The Red Baron posted:

PHP 5.3: now with goto

Pfft, you can't even jump between functions with it.

A A 2 3 5 8 K · # ? Jul 2, 2009 03:22

chips posted:

Who are Dmitry and Sara why are they allowed near PHP?

Who cares if they screw up PHP any further? Think of PHP as a honeypot for keeping stupid crap away from good languages.

Volte · # ? Jul 2, 2009 07:11

I can't see the code but it's definitely a coding horror.

http://forums.somethingawful.com/showthread.php?threadid=3161210

I just sniffed the packets for this game out of curiosity and it sends raw SQL commands back and forth between client and server, as well as updating the SQL backend every 1 second or so for a heartbeat keepalive type thing. :gonk:

gibbed · gibbed Apr 10, 2006

Volte posted:

I can't see the code but it's definitely a coding horror.

http://forums.somethingawful.com/showthread.php?threadid=3161210

I just sniffed the packets for this game out of curiosity and it sends raw SQL commands back and forth between client and server, as well as updating the SQL backend every 1 second or so for a heartbeat keepalive type thing.

I'm not evil but that's just loving stupid. Punish them for it!

Avenging Dentist · # ? Jul 2, 2009 07:59

Vinterstum posted:

Looks like it was for Visual Studio compatibility: http://bulletphysics.com/Bullet/phpBB3/viewtopic.php?f=9&t=3638&p=13749&hilit=std%3A%3Avector#p13749

Oh the HORROR!

Hate to break it to you, but Visual Studio's STL implementation is by Dinkumware and is generally regarded as being pretty good.

(Also if your problem with an implementation of the STL is that it doesn't let you violate the standard then you are probably an idiot.)

Avenging Dentist fucked around with this message at 08:04 on Jul 2, 2009

Vinterstum · # ? Jul 2, 2009 10:40

Avenging Dentist posted:

Hate to break it to you, but Visual Studio's STL implementation is by Dinkumware and is generally regarded as being pretty good.

(Also if your problem with an implementation of the STL is that it doesn't let you violate the standard then you are probably an idiot.)

Strawman much?

1. The Bullet guys needed some of their data to be 16 byte aligned.
2. The VS/Dinkumware std::vector implementation can't handle that.
3. Hence they now use their own array class to be able to remain compatible with VS.

I still fail to see the problem with any of this.

Oh, and the root of the problem is a single function (resize()) in their implementation which passes its second parameter by value instead of by reference, which Microsoft or whomever hasn't bothered to fix for an eternity, and is why quite a lot of people 1) patch the Dinkumware STL themselves, or 2) use a different implementation. But hey, maybe we just shouldn't use those pesky SSE instructions and just stick to the basics.

digibawb · # ? Jul 2, 2009 11:51

Vinterstum posted:

Oh, and the root of the problem is a single function (resize()) in their implementation which passes its second parameter by value instead of by reference, which Microsoft or whomever hasn't bothered to fix for an eternity

Doesn't the standard say it's pass by value?

Sagacity · # ? Jul 2, 2009 12:29

digibawb posted:

Doesn't the standard say it's pass by value?

Which is why they needed to replace it with their own version.

Vinterstum · # ? Jul 2, 2009 12:49

digibawb posted:

Doesn't the standard say it's pass by value?

Actually, yeah it does, though it seems to be a somewhat heavily debated issue (and seems likely to change).

Then I'm unsure why this causes a problem with using SSE intrinsicts only for this implementation and not the others. Edit: Actually, STLPort makes it a const ref.

Anyway, this is a bit of a derail. Back to the real horrors!

Vinterstum fucked around with this message at 12:54 on Jul 2, 2009

digibawb · # ? Jul 2, 2009 13:17

Sagacity posted:

Which is why they needed to replace it with their own version.

I was just questioning the comments on it being "broken", when it does in fact follow the standard. Admittedly, I do agree with the fact that the standard does seem to have "got it wrong" in this case. But yeah, enough of this talk

Avenging Dentist · # ? Jul 2, 2009 18:28

Vinterstum posted:

1. The Bullet guys needed some of their data to be 16 byte aligned.

Yeah cuz writing an allocator is really hard. This isn't even one of those situations where it's a weird allocator like EASTL needed. (The resize thing is an issue but who actually uses vectors anyway? Hurp durp arrays are hard better use a class that allocates up to the next power of two for space so I have amortized constant-time insertion at the end. :saddowns:

Oh also vector<bool> is hilarious.)

There are like a million problems with the STL and it amuses me that when people write their own generic container classes, they solve 1% of the problems, replicate the 99% remaining, and then add a few of their own for flavor.

Avenging Dentist fucked around with this message at 18:39 on Jul 2, 2009

The Noble Nobbler · # ? Jul 3, 2009 15:38

OK, so I've been on a big refactoring kick at work much to my lazy slacker coworkers chagrin. I swear I just found a case of a developer feud personified in code. There are these classes that look like they started out simple enough, then each of them starts subclassing and modifying the base, leading to a clusterfuck of similarly named types repeated in 3 different projects with different interfaces. I thought at most we had two, but this subclassing interfacing war took 4 other classes with it. I spent 3 hours just figuring out what the hell was going on and then separating out the classes.

Oh and then there's the guy who thinks everything is a "manager" so, he subclasses, say, employee, and calls it "employee manager" *adds 4 fields*

Another guy who no longer works here apparently decided to practice his design patterns and created an interface, a virtual base, and then 8 concrete implementations of it so he could

... copy text from a datarow...

TOO SCSI FOR MY CAT · # ? Jul 3, 2009 18:25

"We think there's a bug in this method that reads a file into std::string. Can you look into it, and fix it if it exists? Be careful, that's a very performance-critical method".

It was implemented as:
* Open ifstream
* Copy from file into a streambuf
* Create an istringstream from the streambuf
* Move characters one-by-one from istringstream into vector<char>
* return string(&(the_vector[0]))

And in the end, the error was somewhere else, so I couldn't make any changes :sigh:

Avenging Dentist · # ? Jul 3, 2009 19:03

Janin posted:

And in the end, the error was somewhere else, so I couldn't make any changes

You need to learn how to lie.

TOO SCSI FOR MY CAT · # ? Jul 3, 2009 19:14

Avenging Dentist posted:

You need to learn how to lie.

it's hard to lie when fixing software

"so your changes to this method will fix the bug?"
"yes"
"i've applied them and the bug still happens"

Kelson · Kelson Jan 23, 2005

Janin posted:

it's hard to lie when fixing software

"so your changes to this method will fix the bug?"
"yes"
"i've applied them and the bug still happens"

Two part bug; this makes garbage data and the other makes garbage manipulations.

TOO SCSI FOR MY CAT · # ? Jul 3, 2009 19:35

Kelson posted:

Two part bug; this makes garbage data and the other makes garbage manipulations.

our VCS doesn't have atomic commits, so any bug fixes are an all-or-nothing affair

trex eaterofcadrs · # ? Jul 3, 2009 19:46

Janin posted:

our VCS doesn't have atomic commits, so any bug fixes are an all-or-nothing affair

sounds like quittin' time to me

king_kilr · # ? Jul 3, 2009 19:46

Well, this morning when trying to use a subversion python binding for pyvcs/django-vcs I managed to segfault the interpretter just by attempting to open a client to a local svn repo

Zombywuf · # ? Jul 4, 2009 12:17

Janin posted:

"We think there's a bug in this method that reads a file into std::string. Can you look into it, and fix it if it exists? Be careful, that's a very performance-critical method".

It was implemented as:
* Open ifstream
* Copy from file into a streambuf
* Create an istringstream from the streambuf
* Move characters one-by-one from istringstream into vector<char>
* return string(&(the_vector[0]))

And in the end, the error was somewhere else, so I couldn't make any changes

There's a latent bug in that code that still needs fixing. Also, if it's performance critical that code is another bug. If that isn't enough of an excuse to be allowed to commit it's time to break out the icepick.

Honestly, you're not allowed to commit code to fix horrors?

TOO SCSI FOR MY CAT · # ? Jul 4, 2009 17:35

Zombywuf posted:

There's a latent bug in that code that still needs fixing. Also, if it's performance critical that code is another bug.

The bug's somewhere else, not in that code. Performance issues aren't

Zombywuf posted:

Honestly, you're not allowed to commit code to fix horrors?

"It is what it is, don't make unnecessary changes or something might break"

This is the place with thousand-line long unit tests. Caring about quality just isn't a part of the culture.

They pay well :unsmith:

The Noble Nobbler · # ? Jul 4, 2009 18:05

Janin posted:

The bug's somewhere else, not in that code. Performance issues aren't

"It is what it is, don't make unnecessary changes or something might break"

This is the place with thousand-line long unit tests. Caring about quality just isn't a part of the culture.

They pay well

They have Unit Tests. That's more than most places.

Adbot · # ? Apr 26, 2024 20:15

Zombywuf · # ? Jul 4, 2009 18:49

Janin posted:

The bug's somewhere else, not in that code.

I was talking about zero termination, and the not entirely defined behaviour.