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!

«‹›1503 »

senrath: Nov 4, 2009; Look Professor, a destruct switch!

Technically, it's not only requiring the username to authenticate. That's code to create a new account, from what I can tell. I mean, it's still bad, but not quite that bad.

# ? Mar 30, 2012 22:18

Adbot: ADBOT LOVES YOU

# ? May 14, 2024 03:26

revmoo: May 25, 2006; #basta

Just found my entire company's passwords sitting in plaintext. Sigh.

# ? Mar 30, 2012 22:19

No Safe Word: Feb 26, 2005

revmoo posted:

I'm on thin, thin ice at my company for pointing out SQL injection flaws. I give up. I'm sorry, but saying you can't only validate forms in the browser is not an offensive remark in any way. gently caress.

revmoo posted:

I mentioned in passing that you should probably have server-side validation for a forms infrastructure that runs behind several hundred websites. Apparently development is 'sensitive' and it's offensive to mention things that could or should be done in a different manner. Yesterday I had the director of software development have a meltdown at my desk because I said something about code comments. I'm going to start looking for something else.

revmoo posted:

Just found my entire company's passwords sitting in plaintext. Sigh.

Goddamn dude

# ? Mar 30, 2012 22:26

poemdexter: Feb 18, 2005; Hooray Indie Games!; College Slice

Good luck at your next job revmoo.

# ? Mar 30, 2012 22:29

zeekner: Jul 14, 2007

Get that new job quickly, before they get hacked and that entry on your resume becomes a burden.

# ? Mar 31, 2012 00:47

wwb: Aug 17, 2004

Yeah. Kid writing it swore up and down that he only made it that way for testing. I pointed out that 45 seconds or so with jquery let one make a browser-based post that could test the service properly.

I should also point out that Request.QueryString is a NameValueCollection, which means it returns strings. So the Request.QueryString["FirstName"].ToString() either calls .ToString() on a string or crashes because said value isn't part of the querystring.

It is always quality fun pulling PHP devs into .NET problems.

# ? Mar 31, 2012 00:56

TOO SCSI FOR MY CAT: Oct 12, 2008; this is what happens when you take UI design away from engineers and give it to a bunch of hipster art student "designers"

http://voxel.dl.sourceforge.net/project/malclassifier.adobe/AdobeMalwareClassifier.py

# ? Mar 31, 2012 01:21

Rocko Bonaparte: Mar 12, 2002; Every day is Friday!

That Turkey Story posted:

If you think preprocessor metaprogramming is a horror, you can check out my "horror" contributed to Boost for representing binary literals in C++98/03 and I guess technically C though I haven't tested it:

How'd you get involved in Boost stuff anyways?

# ? Mar 31, 2012 01:38

Dransparency: Jul 19, 2006

Suspicious Dish posted:

It is. GHC segfaulting on terrible code is a feature.

Oh, it can do better than segfault:

Bryan O'Sullivan posted:

The best ghc bug ever involved a dev version of the compiler deleting your source file if it contained a type error.

http://twitter.com/#!/bos31337/statuses/116372971509121025

# ? Mar 31, 2012 02:16

That Turkey Story: Mar 30, 2003

Rocko Bonaparte posted:

How'd you get involved in Boost stuff anyways?

I don't know. When I started using Boost, which was probably 8 years ago or so now, I was immediately amazed at what the libraries were capable of and how they managed to get such simple interfaces, so I started striving for my code to be Boost quality. The release after I started using it, enable_if was added and it completely changed the way I thought about C++. That made me join the dev mailing list, mostly because I wanted to be on the cutting edge. Then I just started participating.

When I added BOOST_BINARY it was sort of by accident. What happened was someone else proposed a template metafunction to represent binary values and it was in the review queue (which, believe it or not, was an even bigger hack than the preprocessor hack). The way it worked was you'd do something like binary< 1001, 0101, 0110 >::value to get the binary value 100101010110. The reason why I say it was even hackier than the macro solution is because you have to notice that 0101 is actually an octal value (since it starts with 0) whereas 1001 is a decimal value, etc. So in order for the metafunction to work, it had a whole slew of weird explicit specializations for decimal and octal values that each converted the corresponding argument to what it would be if the value were binary, then combined all of the arguments together appropriately.

It was cool in that it worked, but it was really sketchy, instantiated a lot of templates, and didn't let you use suffixes or anything, so I asked why they didn't prefer a macro solution and posted some quick, hypothetical syntax. People liked the idea so a few days before the review I implemented the macro, posted it to the mailing list, and it ended up getting pulled into the review along side the template. Ultimately mine was the one that got voted in.

# ? Mar 31, 2012 02:17

wwb: Aug 17, 2004

senrath posted:

Technically, it's not only requiring the username to authenticate. That's code to create a new account, from what I can tell. I mean, it's still bad, but not quite that bad.

Yeah, I'm still trying to wrap my head around the authentication side, though it also works over HTTP get requests. They are also just stuffing raw user IDs in a cookie with zero obfusciation or other defense. Want to be a different user? Just edit your cookie . . . .

# ? Mar 31, 2012 14:33

Toady: Jan 12, 2009

Janin posted:

http://voxel.dl.sourceforge.net/project/malclassifier.adobe/AdobeMalwareClassifier.py

That code is auto-generated.

# ? Mar 31, 2012 21:00

Suspicious Dish: Sep 24, 2011; 2020 is the year of linux on the desktop, bro; Fun Shoe

So, at my last job, we had our own custom data structure: FancyHash. It allowed XPath-style queries on the hash:

code:

fc.get("RESULTS/ANALYSIS/DC");
fc.get("RESULTS[1]/ANALYSIS[2]/DC");

The bracket notation means that there's more than one key with the name RESULTS, and you want the second. The implementation is a giant horror, but I'll just leave you with this:

code:

class KeyVal {
   public String key="";
   public String val="";
   public FancyHash fhVal=null;
   public boolean isLeaf=true;
}

public class FancyHash {

   private Vector<KeyVal> cont=new Vector<KeyVal>();
   private HashMap<String,Vector<FancyHash>> agg=new HashMap<String,Vector<FancyHash>>();
   static private Pattern pattern = Pattern.compile("^([^\\[]*)\\[([^\\]]*)\\]$");

   public String find(String key,int occ) {
      for (KeyVal keyval: cont) { 
          if (keyval.isLeaf && keyval.key.equals(key)) {
             if (occ==0) return(keyval.val);
             else occ--;
          }
      }
      return(null);
   }

   public String get(String path) {
      FancyHash fh=null;
      String tag="";

      int lastInd = path.lastIndexOf('/');
      if (lastInd==-1) {
          fh=this;
          tag=path;
      }
      else {
         String aggpath=path.substring(0,lastInd);
         tag=path.substring(lastInd+1);
         //System.out.println("aggpath=" + aggpath);
         fh=getAgg(aggpath);
      }

      int occ=0;
      Matcher mat = pattern.matcher(tag);
      if (mat.find() && mat.groupCount()==2) { tag=mat.group(1); occ=Integer.parseInt(mat.group(2))-1; }
      //System.out.println("tag=" + tag + ", fh=" + (fh==null));

      if (fh==null) return(null);
      return(fh.find(tag,occ));
   }
}

I left out a bunch of stuff (aggregates), so if you want to see the full horribleness for yourself... Also, yes, this isn't a hash in the slightest - it's O(N) lookup, so let's hope your N is very small...

# ? Mar 31, 2012 21:52

pigdog: Apr 23, 2004; by Smythe

I was trying to understand what the code even does, but my mind started to wander off with all the anal1.put() and anal2.put() lines.

edit: Have to say, putting a test suite in the main[] method of a class is a pretty novel idea. Pretty much exactly like writing real unit tests... except for the part where the user has to verify all the results manually, instead of like, having the computer do it. :monocle:

pigdog fucked around with this message at 22:38 on Mar 31, 2012

# ? Mar 31, 2012 22:28

Suspicious Dish: Sep 24, 2011; 2020 is the year of linux on the desktop, bro; Fun Shoe

The anal1.put and anal2.put lines are just test data for the main function. Aggregates are nested fancy hashes. Here's a JSON equivalent for res1 and res2:

code:

{ "RESULTS": [ { "RETC": [ "0000" ], "RSTYPE": [ "I", "C" ] },
               { "RETC": [ "0001" ], "RSTYPE": [ "I", "C" ] } ] }

So, if you wanted to get the "0001" value above, the query would be RESULTS[1]/RETC. Of course, this is slightly different, since instead of a key pointing to an array, we instead have multiple keys for the same value, so it becomes O(N) lookup.

# ? Mar 31, 2012 22:51

pigdog: Apr 23, 2004; by Smythe

Suspicious Dish posted:

The anal1.put and anal2.put lines are just test data for the main function.

Oh.

# ? Mar 31, 2012 23:11

The Gripper: Sep 14, 2004; i am winner

Suspicious Dish posted:

The anal1.put and anal2.put lines are just test data for the main function. Aggregates are nested fancy hashes. Here's a JSON equivalent for res1 and res2:
code:
{ "RESULTS": [ { "RETC": [ "0000" ], "RSTYPE": [ "I", "C" ] },
               { "RETC": [ "0001" ], "RSTYPE": [ "I", "C" ] } ] }
So, if you wanted to get the "0001" value above, the query would be RESULTS[1]/RETC. Of course, this is slightly different, since instead of a key pointing to an array, we instead have multiple keys for the same value, so it becomes O(N) lookup.

I assume there was some benefit to doing that (maybe input data from somewhere was in that "RESULTS[X]/RETC" format already?), and not just someones decision to write it because it sounded fun?

I enjoy that if someone decided to put or putAgg("RESULT/////////", fh) it'd probably make the entire hash unusable, and peppering \\, <> and ; in it would make both the fcif and XML output incomprehensible because of the weird escaping rules (fcif only escapes ; and \\ with "\\", and xmlescaper is only escaping the Value).

It's just great stuff.

# ? Apr 1, 2012 04:11

pigdog: Apr 23, 2004; by Smythe

Yeah, it's a pretty legit coding horror.

I mean, why would anyone really need such a data structure with such a lookup scheme in the first place? I bet the code that uses that class looks no better.

It was mentioned that performance is a problem, so presumably the actual FancyArray objects are fairly large. Where does the data it is supposed to parse come from?

If the data set is reasonably small, as in it has to fit in memory like a FancyArray would, then why isn't it represented as sensible objects?

If the data set is large enough, then why isn't it stored and queried from a database, and regardless, represented as sensible objects in the code?

It's like, whoever wrote this knew they needed some hierarchical way of storing and accessing data, but didn't really trust this newfangled objects, collections, and encapsulation malarkey.

pigdog fucked around with this message at 06:36 on Apr 1, 2012

# ? Apr 1, 2012 06:34

Carthag Tuek: Oct 15, 2005; Tider skal komme,
tider skal henrulle,
sl�gt skal f�lge sl�gters gang

I bet whoever wrote this saw this or something like it: http://developer.apple.com/library/...SKeyValueCoding

Being able to dig into some object by names in one go is pretty neat, but that impementation is poo poo.

# ? Apr 1, 2012 08:07

pigdog: Apr 23, 2004; by Smythe

I don't know anything about Objective-C or Macs, so can't say what I know the context and what it's good for so well, but from where I'm standing it doesn't seem very neat, either. Perhaps it's a peculiarity of that language and environment, i.e. requirements to support easy scripting, and it can also be considered stable and explicit way of doing things that a developer can rely on and simply needs to learn. But not necessarily something that seems desirable to replicate in normal code or object models.

I mean, in Java you could make an object's fields public and explicitly read/write to them (or use reflection), and there are cases you might even see that as desirable (ie setting up unit tests), but you can also accomplish the same while maintaining encapsulation by setting them to default or protected scope instead of private, and having your unit test live in the same package as the code, though obviously in a separate file and directory.

Meanwhile, even if the class was optimized better, I can't quite imagine the problem to which I'd say "yeah, the best way is to use a FancyHash to do this".

pigdog fucked around with this message at 17:10 on Apr 1, 2012

# ? Apr 1, 2012 09:35

Suspicious Dish: Sep 24, 2011; 2020 is the year of linux on the desktop, bro; Fun Shoe

pigdog posted:

I bet the code that uses that class looks no better.

code:

	private static String prepareBTQueryAttachment(
			SessionDataObject sessionDataObject, String btQueryData) {
		FancyHash queryFH = sessionDataObject.getQueryInfoFH();
		if(btQueryData!=null && queryFH!=null){
			HashMap<String, String> qaMap = new HashMap<String, String>(); 
			if(btQueryData.indexOf("{")==0 && btQueryData.lastIndexOf("}")==btQueryData.length()-1){
				btQueryData = btQueryData.substring(1, btQueryData.length()-1);
			}
			String[] qs = btQueryData.split(",");
			for(int i=0;i<qs.length;i++){
				String key = null;
				String val = null;
				String qsi = qs[i];
				if(qsi.indexOf("\"")==0 && qsi.lastIndexOf("\"")==qsi.length()-1){
					qsi = qsi.substring(1, qsi.length()-1);
					String[] q = qsi.split("\":\"");
					if(q!=null && q.length==2){key=q[0];val=q[1];}
				}
				if(key!=null && val!=null){
					qaMap.put(key, val);
				}
			}
			int qCnt = queryFH.getAggCount("customerQueries/question");
			FancyHash tempFHq =new FancyHash();
			FancyHash btQueryFH =new FancyHash();
			int j=0;
			for(int i=0;i<qCnt;i++){
				FancyHash q = queryFH.getAgg("customerQueries/question["+(i+1)+"]");
				String qId = q.get("questionId");
				if(qaMap.get(qId)!=null){
					FancyHash newq =q.newInstance();
					newq.removeKey("actualAnswer");
					newq.removeKey("questionGroupTimestamp");
					newq.put("actualAnswer",qaMap.get(qId));
					newq.put("questionGroupTimestamp",DtixUtils.convertLocalTimeToUTC());				
					tempFHq.putAgg("question", newq);
				}
			}
			btQueryFH.putAgg("customerQueries", tempFHq);
			return btQueryFH.toXmlString();
		}
		return "";
	}

# ? Apr 1, 2012 15:58

Zhentar: Sep 28, 2003; Brilliant Master Genius

pigdog posted:

I don't know anything about Objective-C or Macs, so can't say what I know the context and what it's good for so well, but from where I'm standing it doesn't seem very neat, either. Perhaps it's a peculiarity of that language and environment, i.e. requirements to support easy scripting, and it can also be considered stable and explicit way of doing things that a developer can rely on and simply needs to learn. But not necessarily something that seems desirable to replicate in normal code or object models.

I mean, in Java you could make an object's fields public and explicitly read/write to them (or use reflection), and there are cases you might even see that as desirable (ie setting up unit tests), but you can also accomplish the same while maintaining encapsulation by setting them to default or protected scope instead of private, and having your unit test live in the same package as the code, though obviously in a separate file and directory.

You don't lose any encapsulation with Key-Value Coding. It still works through the appropriate accessor methods (and you can control a lot of the behavior by overriding certain standard methods). I've personally found it useful for XML [de]serialization, and I'm sure I'll come across more cases.

# ? Apr 1, 2012 19:41

Opinion Haver: Apr 9, 2007

So PHP 5.4 has a web server.

code:

if (!client->request.content) {
    client->request.content = pemalloc(parser->content_length, 1);
    client->request.content_len = 0;
}

Nothing can go wrong here!

# ? Apr 2, 2012 22:45

pokeyman: Nov 26, 2006; That elephant ate my entire platoon.

I'm sure the response is "this is just for development purposes", which completely ignores the fact that some PHP developer thought this was a good idea, and either some other developer signed off on it or they don't review the code at all.

# ? Apr 2, 2012 22:54

wwb: Aug 17, 2004

gently caress me, how does one make sure this piece of poo poo isn't enabled.

# ? Apr 3, 2012 02:04

McGlockenshire: Dec 16, 2005; GOLLOCKS!

It's a CLI thing that has to be expressly enabled when invoking PHP, it can't be daemonized or anything like that.

# ? Apr 3, 2012 07:45

Eggnogium: Jun 1, 2010; Never give an inch! Hnnnghhhhhh!

code:

public static string GetFoo
{
    get
    {
        return _foo;
    }
}

Someone isn't quite grasping the concept of properties. And now I have to add a setter. :what:

# ? Apr 3, 2012 20:19

Dirty Frank: Jul 8, 2004

Oh god, don't do it! :ohdear:

# ? Apr 3, 2012 21:07

New Yorp New Yorp: Jul 18, 2003; Only in Kenya.; Pillbug

Eggnogium posted:

code:
public static string GetFoo
{
    get
    {
        return _foo;
    }
}
Someone isn't quite grasping the concept of properties. And now I have to add a setter.

Rename it!

# ? Apr 3, 2012 21:42

Fiend: Dec 2, 2001

If _foo was public to begin with, then you wouldn't need any fancy property doohickey. Mark everything as public.

# ? Apr 3, 2012 22:03

Lhet: Apr 2, 2008; bloop

Eggnogium posted:

code:
public static string GetFoo
{
    get
    {
        return _foo;
    }
}
Someone isn't quite grasping the concept of properties. And now I have to add a setter.

There might be a chance it was as a hacky way to make a spot for a breakpoint that they forgot to clean up...

On a slightly similar note, today I discovered that my boss, updating an icon for a single window, changed

code:

public static Icon GetAppIcon()
{
  return GlobalInfo.AppIcon;
}

code:

public static Icon GetAppIcon()
{
  return GlobalInfo.FrameIcon;
}

I really never thought the unit test I wrote for that would fail...

# ? Apr 4, 2012 05:54

Beef: Jul 26, 2004

Notch ascended to a new level of coding horrorness:

Removing spaces and using one letter variables makes my code go fasta!

It looks like code copy/pasted from a C hacker that had to fit his code in limited memory.

That or he just copied one of those 'type your own game' books that heavily optimize for code size.

edit: vvvvv Above code being generated makes a bit more sense, indeed.

Beef fucked around with this message at 17:09 on Apr 4, 2012

# ? Apr 4, 2012 12:52

mjau: Aug 8, 2008

Beef posted:

Notch ascended to a new level of coding horrorness:

Removing spaces and using one letter variables makes my code go fasta!

It looks like code copy/pasted from a C hacker that had to fit his code in limited memory.

That or he just copied one of those 'type your own game' books that heavily optimize for code size.

That looks like a 6502 CPU emulator. He's using the standard 6502 register names.

# ? Apr 4, 2012 13:03

The Gripper: Sep 14, 2004; i am winner

mjau posted:

That looks like a 6502 CPU emulator. He's using the standard 6502 register names.

Also I would assume that that particular file is generated, rather than written by hand.

# ? Apr 4, 2012 13:08

ymgve: Jan 2, 2004; Offensive Clock

It's part of Notch's new game, 0x10^c, a space sim/CPU sim hybrid of some sort. Notch is a crazy man.

# ? Apr 4, 2012 13:38

the talent deficit: Dec 20, 2003; self-deprecation is a very british trait, and problems can arise when the british attempt to do so with a foreign culture

there's nothing really wrong with that code, it's a virtual machine. they're all that ugly, and dense and terse is nice in that situation.

# ? Apr 4, 2012 14:21

Beef: Jul 26, 2004

the talent deficit posted:

there's nothing really wrong with that code, it's a virtual machine. they're all that ugly, and dense and terse is nice in that situation.

You are the horror ITT

That snippet being code-generated is an excuse, but being a VM or emulator isn't.

# ? Apr 4, 2012 17:18

PDP-1: Oct 12, 2004; It's a beautiful day in the neighborhood.

Beef posted:

You are the horror ITT

That snippet being code-generated is an excuse, but being a VM or emulator isn't.

I've written my own 6502 emulator and it used giant switch(opcode) statement like the one shown in the screenshot. A switch statement with 256 cases is ugly, but about the best you can do in that situation since there's no logical ordering to the way opcodes are laid out in later-model 6502 chips*. You also have to handle undefined opcodes which may or may not change the PC register while using a variable number of clock cycles.

The real coding horror here is that Notch hasn't figured out that there's only about 40ish logical operations which get mix-n-matched with 16 memory access modes to generate the 200 or so defined opcodes. Instead of hard coding each opcode individually he should be using <logical operation, memory access model> pairs as they'd greatly simplify the code.

*The first 6502 chip did have some logic behind the opcode structure. Each code followed a pattern like aabbbccc where aa was an 'opcode family', bbb was the operation within that family, and ccc was the memory access model used for that specific operation. Later versions of the chip added new codes and memory access models that couldn't be wedged into that naming scheme, so they just said 'gently caress it' and started jamming new instructions in wherever they had a free space. Unfortunately this makes it impossible to write a really simple opcode branching structure in an emulator so you gotta use a giant switch statement.

# ? Apr 4, 2012 17:44

Internet Janitor: May 17, 2008; "That isn't the appropriate trash receptacle."

For comparison, here's somebody else's C implementation based on the spec:
https://github.com/swetland/dcpu16/blob/master/dcpu.c

# ? Apr 4, 2012 19:14

Adbot: ADBOT LOVES YOU

# ? May 14, 2024 03:26

That Turkey Story: Mar 30, 2003

Internet Janitor posted:

For comparison, here's somebody else's C implementation based on the spec:
https://github.com/swetland/dcpu16/blob/master/dcpu.c

Pack it up, Notch.

# ? Apr 4, 2012 19:29

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!

«‹›1503 »