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!

«‹›1501 »

necrotic: Aug 2, 2005; I owe my brother big time for this!

Steve French posted:

it that doesn't have any dumb mistakes

I beg to differ.

# ? Sep 20, 2014 12:27

Adbot: ADBOT LOVES YOU

# ? Apr 19, 2024 09:47

Soricidus: Oct 21, 2010; freedom-hating statist shill

Steve French posted:

Ruby code:

describe ThingRetryWrapper
  let(:thing) { Thing.new }
  let(:thing_retry_wrapper) { ThingRetryWrapper.new(thing) }

  it "retries the right number of times" do
    thing.should_receive(:operation).exactly(3).times.and_raise(SomeError.new("oh noes"))

    expect {
      thing_retry_wrapper.operation
    }.to raise_error
  end
end

(I assume you intended the horror to be Ruby DSLs, but if you had something else in mind then do please feel free to point it out.)

# ? Sep 20, 2014 12:40

qntm: Jun 17, 2009

I used to get that kind of thing in Python because I had no idea what I was doing:

Python code:

def call_which_should_throw_exception():
	pass

try:
	call_which_should_throw_exception()
	assert False
except:
	pass

# ? Sep 20, 2014 13:39

Steve French: Sep 8, 2003

necrotic posted:

I beg to differ.

Heh, I just meant it as a disclaimer for any additional mistakes made when creating this simplified example.

Soricidus posted:

(I assume you intended the horror to be Ruby DSLs, but if you had something else in mind then do please feel free to point it out.)

Nope. Not that I like them, just that that wasn't the point of this particular post. This guy gets it:

qntm posted:

I used to get that kind of thing in Python because I had no idea what I was doing:
Python code:
def call_which_should_throw_exception():
	pass

try:
	call_which_should_throw_exception()
	assert False
except:
	pass

The last little bit should have been:

Ruby code:

expect {
  thing_retry_wrapper.operation
}.to raise_error(SomeError)

But it wasn't, so if the number of actual calls was greater than the number of expected calls, rspec would throw an expectation error to fail the test, and then that last bit would catch it and let the test pass. If the number of actual calls was less than the number of expected calls, rspec would throw an expectation error after the test was finished, and it would not be caught.

So this code is bad for not asserting on the specific type of error expected, but it is also odd that rspec happily catches its own test failure errors. They were smart enough to make the expectation error not subclass StandardError, so it's not caught by the catch-all rescue inside the implementation of operation, but they did not also make the raise_error expectation catch only StandardErrors.

Also a lesser horror: weird mixed use of rspec should and expect syntax.

# ? Sep 20, 2014 17:19

Deus Rex: Mar 5, 2005

I wonder what rspec does if the action under test raises, say, a SyntaxError?

https://github.com/rspec/rspec-expectations/blob/master/lib/rspec/matchers/built_in/raise_error.rb#L42-L49

Ruby code:

# foo.rb
# contains a syntax error
def foo
  {
end

Ruby code:

# foo_spec.rb
describe "foo"
  it "raises something" do
    expect {
      require_relative "./foo"
      foo
    }.to raise_error
  end
end

code:

$ rspec foo_spec.rb
.

Finished in 0.00062 seconds
1 example, 0 failures

# ? Sep 21, 2014 01:08

Steve French: Sep 8, 2003

Deus Rex posted:

I wonder what rspec does if the action under test raises, say, a SyntaxError?

https://github.com/rspec/rspec-expectations/blob/master/lib/rspec/matchers/built_in/raise_error.rb#L42-L49
Ruby code:
# foo.rb
# contains a syntax error
def foo
  {
end
Ruby code:
# foo_spec.rb
describe "foo"
  it "raises something" do
    expect {
      require_relative "./foo"
      foo
    }.to raise_error
  end
end
code:
$ rspec foo_spec.rb
.

Finished in 0.00062 seconds
1 example, 0 failures

Yeah, not passing a specific exception to that expectation is really not a good idea. It's also pretty bad of rspec to let people do that by providing a default value; worse that the default value is Exception.

# ? Sep 21, 2014 04:32

Subjunctive: Sep 12, 2006; ✨sparkle and shine✨

eHow posted:

Design a script to encrypt the credit card information. Using an encryption algorithm of your choice, create a routine that converts the information into encrypted data. Programming languages such as PHP have built in functions that can encrypt. An example is the base 64 encryption function that can be used by invoking the following code:

base64_encode($credit_card_number);

Save this encrypted data in a separate variable.

# ? Sep 21, 2014 22:02

pseudorandom name: May 6, 2007

You didn't really need anything past the "eHow".

# ? Sep 21, 2014 22:19

coffeetable: Feb 5, 2006; TELL ME AGAIN HOW GREAT BRITAIN WOULD BE IF IT WAS RULED BY THE MERCILESS JACKBOOT OF PRINCE CHARLES

YES I DO TALK TO PLANTS ACTUALLY

A few months back Obsidian Entertainment (a games company with a reputation for buggy games) released the beta for their next game. I decompiled it and discovered this monstrosity of a class:

Calling an Obsidian programmer out on this over on another forum, I got

quote:

And?

Skimming this, I don't see anything that doesn't belong. Some classes are light weight, some are not. You could make an argument (from the outside) that some structs could be moved out - but it's just a definition.

Seriously, other than the line count, what do you think is wrong with this? If it was written with old school bracing and spacing, the line count would be around 1000. Line count doesn't mean poo poo.

This class might not even be that heavyweight - you would have to add everything up to see (which I have not done). If for some reason you wanted to - and if there was a reason to, I suppose you could break every last thing out into it's own class file - but there is a cost for that and you aren't really changing anything if Character Stats still needs everything here, because you will still need to include it.

Have you actually gone through and looked for anything technically wrong?

We could go through this file bit by bit, but I would rather play WL2 than argue on the internet. If you don't like the code, that's cool, go write something better - the industry always needs good ideas.

So, my question: is there any way to succinctly explain why this thing is a travesty? I tried writing something up but Christ if I've got to explain why single responsibility and encapsulation are good ideas, I'm not sure there's any hope.

coffeetable fucked around with this message at 22:51 on Sep 21, 2014

# ? Sep 21, 2014 22:48

carry on then: Jul 10, 2010; by VideoGames
(and can't post for 10 years!)

Not exactly sure why you felt the need to decompile a game and try to give a code review to a stranger but okay.

# ? Sep 21, 2014 22:53

hobbesmaster: Jan 28, 2008

That's more like a namespace than a class at that point.

# ? Sep 21, 2014 22:53

coffeetable: Feb 5, 2006; TELL ME AGAIN HOW GREAT BRITAIN WOULD BE IF IT WAS RULED BY THE MERCILESS JACKBOOT OF PRINCE CHARLES

YES I DO TALK TO PLANTS ACTUALLY

carry on then posted:

Not exactly sure why you felt the need to decompile a game

curiosity

quote:

and try to give a code review to a stranger

why do people argue on the internet :iiam:

# ? Sep 21, 2014 22:58

Doctor w-rw-rw-: Jun 24, 2008

carry on then posted:

Not exactly sure why you felt the need to decompile a game and try to give a code review to a stranger but okay.

Obsidian, specifically, is notorious for writing the best drat games ever, but usually ends up releasing pieces of poo poo that take a couple of releases to not crash / not leave the game in an irrecoverable state.

For example, Obsidian had to disable Steam Cloud in New Vegas because it was killing game saves, and Obsidian couldn't figure out how to fix it. On the other hand, after some visual modding it was a blast to play and years after its release, a friend of mine was still finding new and interesting plot interactions in his playthroughs.

# ? Sep 21, 2014 23:06

Brain Candy: May 18, 2006

coffeetable posted:

My God�it's full of stars!

It's an anti-pattern.

This one: http://en.wikipedia.org/wiki/God_object

# ? Sep 21, 2014 23:06

hobbesmaster: Jan 28, 2008

Brain Candy posted:

It's an anti-pattern.

This one: http://en.wikipedia.org/wiki/God_object

Except the large character class isn't a "God object" because it's not controlling the entire program's execution. It's just a store of a ton of information here and that's not too bad as things go.

# ? Sep 21, 2014 23:14

nuvan: Mar 29, 2008; And the gentle call of the feral 3am "Everything is going so well you can't help but panic."

Somehow, after all that, I ended up finding this wonderful thing:
https://web.archive.org/web/20060317085650/http://www.national.com/rap/files/datasheet.pdf

# ? Sep 21, 2014 23:28

Subjunctive: Sep 12, 2006; ✨sparkle and shine✨

The only reasonable way to critique a design is to provide an alternate design. For example, which things don't belong on a character? You could fold a bunch of the attribute-related ones to a fewer parameterized manipulators, but I'm not sure I'd actually prefer that design.

# ? Sep 21, 2014 23:30

Brain Candy: May 18, 2006

hobbesmaster posted:

Except the large character class isn't a "God object" because it's not controlling the entire program's execution. It's just a store of a ton of information here and that's not too bad as things go.

That link I posted posted:

god object is an object that knows too much or does too much.

Note the "knows too much". :3:

If you can't replace an object with a different object you aren't doing OO. That isn't always bad, but megablobs are definitely a bad smell.

# ? Sep 22, 2014 00:20

hobbesmaster: Jan 28, 2008

Brain Candy posted:

Note the "knows too much".

If you can't replace an object with a different object you aren't doing OO. That isn't always bad, but megablobs are definitely a bad smell.

There are likely to be many characters that are being tracked and they could easily subclass that for different types of characters (idk, say expansion character or GM or whatever). Do you think having long application, window or widget classes in a framework is bad too?

# ? Sep 22, 2014 00:31

darkpool: Aug 4, 2014

coffeetable posted:

A few months back Obsidian Entertainment (a games company with a reputation for buggy games) released the beta for their next game. I decompiled it and discovered this monstrosity of a class:

Calling an Obsidian programmer out on this over on another forum, I got

So, my question: is there any way to succinctly explain why this thing is a travesty? I tried writing something up but Christ if I've got to explain why single responsibility and encapsulation are good ideas, I'm not sure there's any hope.

Taken out of context, I feel it's unwieldy and has no clear class to it, instead it tries to cover every class of character within the game it seems. It sucks but I can understand the programmer defending it. However, in context, the class is deriving MonoBehaviour so I guess this is a Unity3D game? A manageable project should be following the entity-component pattern Unity is designed around and lays out for you to follow. It's not a good idea to work against it.

For example, there is a CanCastSpells flag in that class, but the Unity way to do that would be a SpellCaster component which can be added and removed to change the entity's state. To see if the entity is in a spell casting state, the existence of the SpellCaster component is checked rather than doing it through a fixed member.

# ? Sep 22, 2014 00:53

down with slavery: Dec 23, 2013; STOP QUOTING MY POSTS SO PEOPLE THAT AREN'T IDIOTS DON'T HAVE TO READ MY FUCKING TERRIBLE OPINIONS THANKS

Brain Candy posted:

If you can't replace an object with a different object you aren't doing OO.

If you exclusively program in OO you are a bad programmer.

# ? Sep 22, 2014 01:29

hobbesmaster: Jan 28, 2008

down with slavery posted:

If you exclusively program in OO you are a bad programmer.

More smalltalk discrimination.

# ? Sep 22, 2014 01:32

Flownerous: Apr 16, 2012

Yeah that's pretty typical for a game character class in any game let alone an RPG. You can bet each function in that class ended up reading from and writing to many aspects of the character, and that those rules were probably changing every day during the pre-production phase when they were experimenting with how the game would play.

Sometimes you get a chance to go back and refactor out anything that can be, but that's rare when there are bugs that will actually impact consumers to fix.

# ? Sep 22, 2014 01:39

hobbesmaster: Jan 28, 2008

And if you really think you can do better... http://www.obsidian.net/jobs/open-positions/programming/447-engine-programmer

# ? Sep 22, 2014 01:48

ErIog: Jul 11, 2001

The fact that there was any response from the developer other than "Please stop decompiling our game code. If you would like to file a specific bug report, please do so at <URL>" is the true horror. Bothering to engage with people giving unsolicited code reviews is just a giant can of worms, and doing so on a forum just leads to endless bikeshedding.

As well, if this is a Unity game then nearly any horror a dev is going to construct in Unity is eclipsed by the horror that is Unity itself.

ErIog fucked around with this message at 02:18 on Sep 22, 2014

# ? Sep 22, 2014 02:15

Space Kablooey: May 6, 2009

ErIog posted:

As well, if this is a Unity game then nearly any horror a dev is going to construct in Unity is eclipsed by the horror that is Unity itself.

I'm not doubting that Unity isn't garbage, but is there any articles or posts that goes in detail on why it's garbage?

Also, while searching for any article like that, I found this:

forum.unity3d.com/threads/programming-languages-considered-harmful.18009/

:stare:

# ? Sep 22, 2014 03:05

QuarkJets: Sep 8, 2008

HardDisk posted:

I'm not doubting that Unity isn't garbage, but is there any articles or posts that goes in detail on why it's garbage?

Also, while searching for any article like that, I found this:

forum.unity3d.com/threads/programming-languages-considered-harmful.18009/

Come on, he created a computer game 15 years ago for a friend. That means that his opinion is insightful

# ? Sep 22, 2014 03:26

ErIog: Jul 11, 2001

HardDisk posted:

I'm not doubting that Unity isn't garbage, but is there any articles or posts that goes in detail on why it's garbage?

Also, while searching for any article like that, I found this:

forum.unity3d.com/threads/programming-languages-considered-harmful.18009/

You can read about all the problems with Unity here:
http://forums.somethingawful.com/showthread.php?threadid=2692947

I think a lot of discussion of Unity problems started around page like 305 or something, and gets reignited whenever there's some new announcement about it.

The main issues stem from them trying to solve every problem by throwing more programmers at it, and they chose to fork Mono instead of just licensing the existing version. This might have been fine if they had been diligent about updating it, but they haven't at all. So it lags behind a zillion years. They announced plans to try to fix these issues in May, but those plans, unsurprisingly, involve tasking more programmers with reinventing more wheels.

Then you compare it to something like Unreal Engine, and you just start to wonder what they're thinking.

Here's the blog post where they lay out their master plans for fixing the issues that stem from their tremendously out of date Mono fork:
http://blogs.unity3d.com/2014/05/20/the-future-of-scripting-in-unity/

Most of the ire toward Unity in the game dev community is somewhat recent, though. So I wouldn't really blame anyone for having picked it for their project that's been in development for a long time. If a dude's got time to decompile and bitch about what he finds online then the Unity engine itself a much more target-rich environment for horrors.

ErIog fucked around with this message at 04:10 on Sep 22, 2014

# ? Sep 22, 2014 04:01

SupSuper: Apr 8, 2009; At the Heart of the city is an Alien horror, so vile and so powerful that not even death can claim it.

coffeetable posted:

A few months back Obsidian Entertainment (a games company with a reputation for buggy games) released the beta for their next game. I decompiled it and discovered this monstrosity of a class:

*snip*

Calling an Obsidian programmer out on this over on another forum, I got

So, my question: is there any way to succinctly explain why this thing is a travesty? I tried writing something up but Christ if I've got to explain why single responsibility and encapsulation are good ideas, I'm not sure there's any hope.

It's a D&D CRPG. Every character is gonna have a massive character sheet tied to their backs (note that it's a class dedicated solely to CharacterStats, not even getting into the Character). While it's possible you could segregate stuff like bonuses and effects and such into subclasses, there's no guarantee the game rules won't need them all to do calculations anyways, making it a very inter-dependent component system.

Edit: If you want Unity horrors, there's plenty of operator overloading, hidden side-effects and many more abuses of the C# language (all running on a dated and customized Mono interpreter which is incompatible with most outside libs, naturally).

SupSuper fucked around with this message at 04:12 on Sep 22, 2014

# ? Sep 22, 2014 04:07

QuarkJets: Sep 8, 2008

SupSuper posted:

It's a D&D CRPG. Every character is gonna have a massive character sheet tied to their backs (note that it's a class dedicated solely to CharacterStats, not even getting into the Character). While it's possible you could segregate stuff like bonuses and effects and such into subclasses, there's no guarantee the game rules won't need them all to do calculations anyways, making it a very inter-dependent component system.

Edit: If you want Unity horrors, there's plenty of operator overloading, hidden side-effects and many more abuses of the C# language (all running on a dated and customized Mono interpreter which is incompatible with most outside libs, naturally).

Hope you don't accidentally call TriggerWhenHit() instead of TriggerWhenHits()

I guess that it's possible that the classes listed here aren't all-encompassing. It would be really swell to have classes for races like Orcs and Humans, and then classes for professions like Wizard and Barbarian, that all inherit from BaseStats. It's totally possible that that's actually what has been done, and we just haven't been shown those class definitions.

We can hope

QuarkJets fucked around with this message at 07:52 on Sep 22, 2014

# ? Sep 22, 2014 07:44

coffeetable: Feb 5, 2006; TELL ME AGAIN HOW GREAT BRITAIN WOULD BE IF IT WAS RULED BY THE MERCILESS JACKBOOT OF PRINCE CHARLES

YES I DO TALK TO PLANTS ACTUALLY

i can't put together a proper reply this morning because ive got work and this

Subjunctive posted:

The only reasonable way to critique a design is to provide an alternate design.

is a really good point, but

QuarkJets posted:

It would be really swell to have classes for races like Orcs and Humans, and then classes for professions like Wizard and Barbarian, that all inherit from BaseStats.

lol no

http://pastebin.com/PgtgPDPC

# ? Sep 22, 2014 08:15

Athas: Aug 6, 2007; fuck that joker

hobbesmaster posted:

Except the large character class isn't a "God object" because it's not controlling the entire program's execution. It's just a store of a ton of information here and that's not too bad as things go.

Well, it does seem to contain a huge number of enums for things like (character) classes, races and cultures. Some of these should probably be proper independent classes, as this smacks of some ugly type-case/switch statement somewhere. Or best of all: move that stuff to data files loaded at runtime, rather than baking it statically into the code.

# ? Sep 22, 2014 08:16

QuarkJets: Sep 8, 2008

coffeetable posted:

i can't put together a proper reply this morning because ive got work and this

is a really good point, but

lol no

http://pastebin.com/PgtgPDPC

Well then I retract my statement. An RPG game is probably the most obvious use case for class inheritance.

# ? Sep 22, 2014 08:22

Edison was a dick: Apr 3, 2010; direct current only

coffeetable posted:

A few months back Obsidian Entertainment (a games company with a reputation for buggy games) released the beta for their next game. I decompiled it and discovered this monstrosity of a class:

http://i.imgur.com/RO7r8QP.png

Calling an Obsidian programmer out on this over on another forum, I got

So, my question: is there any way to succinctly explain why this thing is a travesty? I tried writing something up but Christ if I've got to explain why single responsibility and encapsulation are good ideas, I'm not sure there's any hope.

The video http://vimeo.com/9270320 makes a lot of points and actually has researched them. One of the conclusions that lines of code is the best metric that actually works, the more you have, the more bugs you have, and humans can't really manage more than 200 lines of code at once.
The fellow who wrote that class is the only person who can possibly understand it.

There's also a less well researched article called Norris Numbers which is more about total lines of code, and how if you've got a big project, you really need good abstractions to be able to work with it as soon as it gets beyond 2000 lines of code.

# ? Sep 22, 2014 09:02

Jabor: Jul 16, 2010; #1 Loser at SpaceChem

Honestly the big stack of fields and properties looks fine to me. Yeah, you could split it up into a bunch of highly-interconnected nested classes, but all that that would achieve is making it a bigger pain in the rear end to tweak things when the design changes.

The bigger issue is all those methods - a lot of them would be better served in logically-separate utility classes (the level-up stuff, for example). If that's what the OP was talking about they should probably have just put in a screenshot of the methods instead of the whole thing, because the big stack of fields and properties looks a whole lot like necessary complexity, and leading with it makes it look as if you're saying "big = bad" and don't actually really know what you're talking about.

# ? Sep 22, 2014 09:46

M31: Jun 12, 2012

QuarkJets posted:

Well then I retract my statement. An RPG game is probably the most obvious use case for class inheritance.

It's seems like a good fit, but in the real world it doesn't work. You can make an Orc class, but then you get an PlayableOrc and a NonPlayableOrc, and the NonPlayableOrc has a friendly version and a non friendly version, etc.. And C# does not have multiple inheritance.

Composition is much more maintainable and having a standard class with all character attributes seems pretty logical to me in that context, even if you have a gazillion attributes.

# ? Sep 22, 2014 09:47

TheresaJayne: Jul 1, 2011

coffeetable posted:

A few months back Obsidian Entertainment (a games company with a reputation for buggy games) released the beta for their next game. I decompiled it and discovered this monstrosity of a class:

SNIP...

Calling an Obsidian programmer out on this over on another forum, I got

So, my question: is there any way to succinctly explain why this thing is a travesty? I tried writing something up but Christ if I've got to explain why single responsibility and encapsulation are good ideas, I'm not sure there's any hope.

Throw them a link to Clean Code by Uncle Bob - that might be helpful, or why not a link to SOLID
S ingle Responsibility Principle
O pen Close Principle
L iskov Substitution Principle
I nterface Segregation Principle
D ependency Injection

# ? Sep 22, 2014 10:19

Maluco Marinero: Jan 18, 2001; Damn that's a
fine elephant.

M31 posted:

It's seems like a good fit, but in the real world it doesn't work. You can make an Orc class, but then you get an PlayableOrc and a NonPlayableOrc, and the NonPlayableOrc has a friendly version and a non friendly version, etc.. And C# does not have multiple inheritance.

Composition is much more maintainable and having a standard class with all character attributes seems pretty logical to me in that context, even if you have a gazillion attributes.

I haven't devved a game, but it really looks like inheritance creates more problems than it solves, especially if your toolset only has single inheritance. If you must use classes you'd preferably have some sort of multiple inheritance system like mixins, Scala's traits, etc. Feels like the more coupling in your class structure, the more painful balancing may become once the game is of adequate complexity, let alone finding/fixing bugs.

From the outside looking in it feels like Entity Component System are the best way to represent game worlds in a decoupled way, is that correct or are there better approaches?

# ? Sep 22, 2014 10:44

pigdog: Apr 23, 2004; by Smythe

coffeetable posted:

A few months back Obsidian Entertainment (a games company with a reputation for buggy games) released the beta for their next game. I decompiled it and discovered this monstrosity of a class:

So, my question: is there any way to succinctly explain why this thing is a travesty? I tried writing something up but Christ if I've got to explain why single responsibility and encapsulation are good ideas, I'm not sure there's any hope.

Yeah, I don't think you have much ground to stand on here. It does have single responsibility and encapsulation, there's just a lot of stuff there because it's an RPG game and characters in a RPG have a lot of stuff involving them. Sure you could hack it into smaller classes, but that wouldn't necessarily make it cleaner, easier, or more logical to use. There's just a lot fields, because there needs to be, and a lot of code involving these fields, because code dealing with these fields may best be associated with the object that has them.

# ? Sep 22, 2014 10:44

Adbot: ADBOT LOVES YOU

# ? Apr 19, 2024 09:47

Soricidus: Oct 21, 2010; freedom-hating statist shill

M31 posted:

Composition is much more maintainable and having a standard class with all character attributes seems pretty logical to me in that context, even if you have a gazillion attributes.

This is the first time I've heard "gently caress it, let's just indiscriminately throw everything into a single class" described as composition.

# ? Sep 22, 2014 10:48

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!

«‹›1501 »