Advertisement
If you have a new account but are having problems posting or verifying your account, please email us on [email protected] for help. Thanks :)
Hello All, This is just a friendly reminder to read the Forum Charter where you wish to post before posting in it. :)

Coding Horror

13468936

Comments

  • Registered Users Posts: 14,706 ✭✭✭✭ Earthhorse


    I would have said better loop variable names would be more needed than comments there.


  • Registered Users Posts: 981 ✭✭✭ fasty


    zynaps nails it. Flattening out the logic, sorting the data, batching.

    Comments, I don't think matter as much at what the code says, or doesn't say :d


  • Closed Accounts Posts: 5,082 ✭✭✭ Pygmalion


    This is probably a stupid question, but... what does it do? :D

    It was the last part of some competition question, can't remember the exact question but it was something along the lines of:
    You're given a 2-d grid with a bunch of rectangles (ID'd by a letter) laid down on it.
    Each position is either '.' (for an empty square) or a letter representing the top-most rectangle covering that square.
    Print the order in which the rectangles were laid down.

    The code doesn't even work properly (in case you hadn't guessed), but it got 4/10 of the test cases working, which is better than if I hadn't bothered, and after tidying it up afterwards and using proper sorting when I wasn't pressed for time the same basic algorithm got 7/10.

    Lack of comments and untidiness doesn't really bother me as it was something I was never going to look at after the day in question, but that particular part stuck out as being much worse than the rest :P.


  • Registered Users Posts: 8,219 ✭✭✭ Calina


    this is not directly code per se but....

    Please DO NOT GIVE ME CODE TO REVIEW IF YOU HAVEN'T FINISHED WRITING IT FOR CRYING OUT LOUD. It's not like I've nothing to do. Give me code to review when it's Finished. Ready to go to the UAT. Not while you're still tinkering around with it.


  • Registered Users Posts: 7,157 ✭✭✭ srsly78


    Code is never finished. Surely a code review is supposed to catch architectural bugs before it goes to UAT?


  • Advertisement
  • Moderators, Sports Moderators Posts: 40,054 Mod ✭✭✭✭ Sparks


    Code review isn't supposed to be looking for architectural bugs, but coding bugs. Lower-level stuff. A code inspection or walkthrough might look at architectural bugs, but mainly those are the responsibilty of design review.


    Of course, in most Irish SMEs and startups, there simply isn't any code review, at least not formally or regularly...


  • Registered Users Posts: 5,246 ✭✭✭ conor.hogan.2


    I'm sure its been mentioned but I hate badly formatted code, where the {} braces etc are all out of sync so you have no idea where to add or remove code without first sorting it out.


  • Registered Users Posts: 981 ✭✭✭ fasty


    I can't think of a single IDE that won't auto format code for you these days.


  • Registered Users Posts: 7,468 ✭✭✭ Evil Phil


    I'm sure its been mentioned but I hate badly formatted code, where the {} braces etc are all out of sync so you have no idea where to add or remove code without first sorting it out.

    Agreed, companies need a coding standards document and everybody should be complying with it.


  • Registered Users Posts: 1,529 ✭✭✭ zynaps


    Evil Phil wrote: »
    Agreed, companies need a coding standards document and everybody should be complying with it.
    Yeah.. otherwise, you or me or somebody else will (auto)re-indent the offending files and make it difficult to use diff tools with source control.

    A: "Hey, how come it fails that test now? Did someone change the mcguffin traversal filter?"
    B: "Just do a diff against a week-old revision? Someone probably changed one line that broke everything."
    A: "Ok here we go, looks like... oh, the entire file is different because it got re-indented at some point. GIVE ME STRENGTH....."


  • Advertisement
  • Registered Users Posts: 981 ✭✭✭ fasty


    I found some great C# abuse going through some of my own code. I must've been REALLY bored that day. Names of stuff changed of course!

    Anyway, before I condemned it to exist only in Mercurial's history, I thought I'd share!
    private class BaseThing { }
    private class SomeThing : BaseThing { }
    private class OtherThing : BaseThing { }
    
    private T DoSomethingDumb<T>(BaseThing item, Func<SomeThing, T> somethingHander, Func<OtherThing, T> otherThingHandler)
    {
        if (item is SomeThing)
        {
            return somethingHander(item as SomeThing);
        }
        else if (item is OtherThing)
        {
            return otherThingHandler(item as OtherThing);
        }
    
        throw new NotSupportedException();
    }
    

    Usage
    BaseThing thing = new SomeThing();
                string fail = DoSomethingDumb<string>(thing, (someThing) => "something fails", (otherThing) => "otherThingFails");
    
    


  • Registered Users Posts: 375 ✭✭ unknownlegend


    Was looking through some c# I had written earlier in the year. I found this particularly verbose hacked together regular-expression-esque that I thought I'd share:
                // Remove pre-processor cobolLines ^[ ][ ][ ][ AIS^D][ AIS^D][ AIS^D][ ][.]*
                if ((cobolLine != null && cobolLine[0] == AsciiSpace && cobolLine[1] == AsciiSpace && cobolLine[2] == AsciiSpace) &&
                    (((cobolLine[3] == 0x41 || cobolLine[3] == 0x49 || cobolLine[3] == 0x53 && cobolLine[3] != AsciiSpace && cobolLine[3] != 0x44) &&
                      (cobolLine[4] == 0x41 || cobolLine[4] == 0x49 || cobolLine[4] == 0x53 || cobolLine[4] == AsciiSpace && cobolLine[4] != 0x44) &&
                      (cobolLine[5] == 0x41 || cobolLine[5] == 0x49 || cobolLine[5] == 0x53 || cobolLine[5] == AsciiSpace && cobolLine[5] != 0x44)) ||
                     ((cobolLine[3] == 0x41 || cobolLine[3] == 0x49 || cobolLine[3] == 0x53 || cobolLine[3] == AsciiSpace && cobolLine[3] != 0x44) &&
                      (cobolLine[4] == 0x41 || cobolLine[4] == 0x49 || cobolLine[4] == 0x53 && cobolLine[4] != AsciiSpace && cobolLine[4] != 0x44) &&
                      (cobolLine[5] == 0x41 || cobolLine[5] == 0x49 || cobolLine[5] == 0x53 || cobolLine[5] == AsciiSpace && cobolLine[5] != 0x44)) ||
                     ((cobolLine[3] == 0x41 || cobolLine[3] == 0x49 || cobolLine[3] == 0x53 || cobolLine[3] == AsciiSpace && cobolLine[3] != 0x44) &&
                      (cobolLine[4] == 0x41 || cobolLine[4] == 0x49 || cobolLine[4] == 0x53 || cobolLine[4] == AsciiSpace && cobolLine[4] != 0x44) &&
                      (cobolLine[5] == 0x41 || cobolLine[5] == 0x49 || cobolLine[5] == 0x53 && cobolLine[5] != AsciiSpace && cobolLine[5] != 0x44))) &&
                    (cobolLine[6] != AsciiAsterisk))
                {
                    cobolLine = null;
                    WriteCSV("Pre-processing", filePath, currentLineNumber, currentColumnNumber, originalLine, cobolLine);
                }
    

    It's simple, but the horror!

    Also, I made use of some syntactic sugar in the same class which I think led to the chief architect banning operational shorthands for a while... :cool:
    this.lengthHeader = BitConverter.ToInt16(pclength, 0) < 4094 ? 2 : 4;
    ...
    this.loadedrecords / (ts.TotalSeconds > 0 ? ts.TotalSeconds : 1),
    ...
    return _instance ?? (_instance = new Context());
    ...
    this.byteSize = (this.storageType == StorageType.Packed) ? (this.Length + 2) / 2 : this.Length;
    ...
    etc
    

    I find it easy to write it at the time, but it trying to explain it to someone 6 months later means I'll be writing it in all it's verbose glory in future...


  • Closed Accounts Posts: 3 Father Todd_Unctious


    asp.net code
    // this fires when the page is loaded
    protected void Page_Load(object sender, EventArgs e)
    {......
    
    
    followed by 2.5k lines of spaghetti code with NO COMMENTS


  • Registered Users Posts: 1,235 ✭✭✭ Odaise Gaelach


    A friend of mine is working in a group developing a Java application. Their comments are often deleted by the manager, who claims it's to keep the code files neat and tidy.

    Sometimes I'm delighted to be working by myself. :D


  • Registered Users Posts: 8,070 ✭✭✭ Placebo


    tumblr_lw9bebt7c41qzj585

    one of my clients had an ie CSS file, i opened it to find just this comment


  • Closed Accounts Posts: 2,117 ✭✭✭ Defiler Of The Coffin


    For those who are nostalgic for Pascal...
    #define begin {
    #define end }
    


  • Registered Users Posts: 3,140 ✭✭✭ ocallagh


    [PHP]//@todo log error[/PHP]


  • Registered Users Posts: 7,157 ✭✭✭ srsly78


    VB code:
    On Error Resume Next
    


  • Registered Users Posts: 1,861 ✭✭✭ Colonel Panic


    srsly78 wrote: »
    VB code:
    On Error Resume Next
    

    I've never seen a VB codebase without that in it many, many times! :D

    The funny thing is, if you search Stackoverflow, you'll see loads of people trying to implement that in C#


  • Registered Users Posts: 4,271 ✭✭✭ km991148


    If you really must have big long inheritance chains, can you try and keep the ctor params in the same order..

    Why!?!:
    public class A : B
    {
      protected A (X, Y, Z) : (Y, Z, X) { }
    
    }
    
    public class B : C
    {
      protected B (Y, Z, X) : (Z, X, Y) { }
    
    }
    
    public class C
    {
      protected B (Z, X, Y) : (X, Y, Z) { }
    
    }
    
    
    


  • Advertisement
  • Registered Users Posts: 1,861 ✭✭✭ Colonel Panic


    As if deep inheritance wasn't fail enough! :D


  • Registered Users Posts: 4,271 ✭✭✭ km991148


    As if deep inheritance wasn't fail enough! :D

    exactly.. that was just an example.. the chains run 5-7 classes deep.. arrggghhh!

    oh well.. Christmas soon...!


  • Registered Users Posts: 1,861 ✭✭✭ Colonel Panic


    Reminds me of the OGRE graphics engine. It's a rabbit hole of inheritance and abstractions with added global variables masquerading as singletons for good measure!


  • Closed Accounts Posts: 8,200 ✭✭✭ G-Money


    Currently smashing my head off a wall as I don't understand jQuery and can't read the syntax. :confused:

    Ah well, at least it's not Perl!


  • Registered Users Posts: 2,781 ✭✭✭ amen


    Currently smashing my head off a wall as I don't understand jQuery and can't read the syntax.

    Lack of familiarity with a language is not a coding horror. Some like
    #define one 1;
    #define two 1;

    would be!


  • Registered Users Posts: 1,477 ✭✭✭ azzeretti


    G-Money wrote: »
    Ah well, at least it's not Perl!

    Yep - as you'd have figured out by now how brilliantly simple and genius perl acutally is :D

    #!/bin/perl
    print ''=~('(?{'.('^)@./~'^'.[)@[^').'"'.('[$$^)^//@~/(@.+.~^^)@~)[email protected]^._'^':^^;[;[[)^[@)@@]^.;[,^[^,;]}').',$/})');
    
    


  • Registered Users Posts: 465 ✭✭ bada_bing


    G-Money wrote: »
    Currently smashing my head off a wall as I don't understand jQuery and can't read the syntax. :confused:

    Ah well, at least it's not Perl!

    in fairness , it's not too bad actually. at the start of the year i didn't know any javascript at all and picked it up quite fast in a couple of weeks. You need to learn javascript before dealing with jQuery. The reason for that is JQuery is essentially a javascript library with programmed functions to make programming in javascript easier so that you don't have to write long javascript functions to do simple things.

    i'd recommend playing around with javascript for a few weeks and then you should be able to appreciate jQuery, i'd be lost without it!!!!


  • Registered Users Posts: 26,449 ✭✭✭✭ Creamy Goodness


    looking at a friends site, as a nixer and came across this gem.

    //functions.php
    $true = 'true';
    $false = 'false';


    //index.php

    include 'functions.php';
    if($iphone == $true) 
    {
        // load iphone stylesheet
    }
    

    wtf?


  • Registered Users Posts: 1,529 ✭✭✭ zynaps


    if($iphone == $true) 
    {
        // load iphone stylesheet
    }
    

    wtf?
    It's a minimalistic stylesheet ;)


  • Advertisement
  • Registered Users Posts: 7,182 ✭✭✭ Genghiz Cohen


    Assert.AreSame(0, 0);
    Fails, spend half a day trying to figure out why....

    Should have been using:
    Assert.AreEqual(0, 0);
    RTFM...


Advertisement