Rule implementations



  • One aspect of our system is rule driven. The rules engine then calls some Java function with the parameters from the input system to get a value that is to be used as part of the result of evaluating the rule.

    Since the guy who usually does this is on an extended vacation and unreachable for a few weeks, I was asked to make a change to one of the rules.

    I dug, and dug, and dug, and dug, and finally found the imlementation file, conveniently named with what appears to be a random series of characters with no discernable meaning that anyone can determine. I opened the file in eclipse on a 16 chip 64 core 3.2 gHz machine. And I wait. Long enough to open a window and type: top.

    Eclipse was grinding through the file trying to build its internal tables.

    Then the file finally opened.

    We have thousands of rules. The file had thousands of functions. Each and every one being the EXACT SAME ~120 LOC with only minor differences:

     

       public static int evalFuncForRuleN(same 50ish args for every function) {
    // 90 LOC identical in each and every function
    int returnCode = n; //OP: n is the same as in the function name
    // 30 LOC nearly identical in each and every function
    return returnCode;
    }

    The only differences were the number of the function, and the strings in the various error messages. The rest of the logic did a series of if-tests to see if parameter-x should be used. Everything was passed to every function whether it was needed or not.

    Some folks who were here when this guy first started writing this mess told me that he didn't understand function overloading or virtual functions, so he made every function a cut-n-paste job.

    I refactored it into thousands of tiny classes with a couple of overloaded methods that just returned the appropriate int or string, reduced the whole mess to about 120 LOC, ran all the test cases and committed it to source control.

    Honestly, I can understand someone not knowing function overloading and the concept of virtual functions, but the folks who saw what he was doing absolutely knew better and chose to do nothing.

     



  • How did you implement "thousands of tiny classes" in just 120 lines of code???


  • Considered Harmful

    @TheCPUWizard said:

    How did you implement "thousands of tiny classes" in just 120 lines of code???


    No linebreaks in the file except for those forced by the line length limits of your IDE.



  • @TheCPUWizard said:

    How did you implement "thousands of tiny classes" in just 120 lines of code???

    Simple: By deleting the thousands of classes and replacing them with only the number that was actually needed.

  • Considered Harmful

    @snoofle said:

    same 50ish args for every function

    Bonus WTF (I would say TRWTF but, no, the same 100+ lines copypasta'd thousands of times is worse)



  • @TheCPUWizard said:

    How did you implement "thousands of tiny classes" in just 120 lines of code???

    Maybe I could have been clearer. The hundred+K LOC was reduced to 120 in a base class. The  rest was subclassed in stuff like this:

      public class ClassForRuleN extends AbstractBaseClass {
        public int ruleNumber() { return n; } // the actual rule number
        public String ruleName() { return "..."; }
        public long getControlMaskBitFlags() { return 0x...; }
      }
    

    I had the option of specifying the constants in the rules file, but then nobody except the guy who was supposed to be doing it would be able to work with it. This way, none of the subclasses will ever change, and whenever a change has to be made to the base logic, it'll only need to be made in one place (instead of thousands).

    For me, the royal PITA was going through the rules themselves and changing the numeric names to the names of the rule sub-classes (I used the actual human-readable names instead of 1..n, so it took me a while to do the editing, but at least we'll never need to deal with this crap again).



  • @snoofle said:

    @TheCPUWizard said:

    How did you implement "thousands of tiny classes" in just 120 lines of code???

    Maybe I could have been clearer. The hundred+K LOC was reduced to 120 in a base class. The  rest was subclassed in stuff like this:

      public class ClassForRuleN extends AbstractBaseClass {
        public int ruleNumber() { return n; } // the actual rule number
        public String ruleName() { return "..."; }
        public long getControlMaskBitFlags() { return 0x...; }
      }
    I pretty much figured that... But for a real LOC based comparision, it is needed to count the 5 lines of each of the thousands of classes.
    Your work is a MAJOR improvement.. but I have a pet peeve when statistics about improvements get skewed.


  • @TheCPUWizard said:

    @snoofle said:

    @TheCPUWizard said:

    How did you implement "thousands of tiny classes" in just 120 lines of code???

    Maybe I could have been clearer. The hundred+K LOC was reduced to 120 in a base class. The  rest was subclassed in stuff like this:

      public class ClassForRuleN extends AbstractBaseClass {
        public int ruleNumber() { return n; } // the actual rule number
        public String ruleName() { return "..."; }
        public long getControlMaskBitFlags() { return 0x...; }
      }
    I pretty much figured that... But for a real LOC based comparision, it is needed to count the 5 lines of each of the thousands of classes.
    Your work is a MAJOR improvement.. but I have a pet peeve when statistics about improvements get skewed.

    Fair enough. Around here, the metric they use is: don't count imports and declarations, lines with just one brace, blank lines or comment lines.

    FWIW, I wrote a script to (crudely) parse the rules tokenized file and generate all the subclasses because I wasn't spending days typing all of that.

     



  • And while you're busy 'fixing' things, that coder who was paid by the line is off enjoying his hovercraft(s).


  • Considered Harmful

    @db2 said:

    And while you're busy 'fixing' things, that coder who was paid by the line is off enjoying his hovercraft(s).

    I thought I paid you to keep quiet about my hovercrafts!



  • @snoofle said:

      public class ClassForRuleN extends AbstractBaseClass {
    public int ruleNumber() { return n; } // the actual rule number
    public String ruleName() { return "..."; }
    public long getControlMaskBitFlags() { return 0x...; }
    }

    Snoofle, this sounds like a good candidate for a lookup table: a dictionary, keyed off of "n"; with a value consisting of a tuple with ruleNumber, ruleName, controlMaskBitFlags.

    Thusly elminating the thousands of tiny classes (unless the code requires all these subclasses to be floating around?)


  • Considered Harmful

    That wouldn't be enterprisey at all.



  • @snoofle said:

    One aspect of our system is rule driven. The rules engine then calls some Java function with the parameters from the input system to get a value that is to be used as part of the result of evaluating the rule.

    Since the guy who usually does this is on an extended vacation and unreachable for a few weeks, I was asked to make a change to one of the rules.

    ...

    I refactored it into thousands of tiny classes with a couple of overloaded methods that just returned the appropriate int or string

    So what's the likelihood this guy is going to freak the hell out when he gets back and sees everything changed?



  • @CodeNinja said:

    So what's the likelihood this guy is going to freak the hell out when he gets back and sees everything changed?

    So what's the likelyhood this guy learns something about coding when he gets back?


  • ♿ (Parody)

    @joe.edwards said:

    '
    That wouldn't be enterprisey at all.

    It will be after they serialize it all into xml and then stuff it into a database.



  • @DrPepper said:

    @CodeNinja said:

    So what's the likelihood this guy is going to freak the hell out when he gets back and sees everything changed?

    So what's the likelyhood this guy learns something about coding when he gets back?

    99.973% and 0.005%, respectively. This is someone who can copy-paste 120,000 lines of code and not think "there has to be a better way". Being shown a better way is unlikely to change anything.



  • @Ibix said:

    not think "there has to be a better way"
     

    Maybe he did, but thought git 'er done had the greater priority. And then there was the next project. That's how it goes.



  • @dhromed said:

    @Ibix said:

    not think "there has to be a better way"
     

    Maybe he did, but thought git 'er done had the greater priority. And then there was the next project. That's how it goes.

    was asked to make a change to one of the rules.

    Software is not done until the last running instance has been removed from production, and it is demonstrably proven that the data will never be needed again!



  • @DrPepper said:

    @snoofle said:
      public class ClassForRuleN extends AbstractBaseClass {
    public int ruleNumber() { return n; } // the actual rule number
    public String ruleName() { return "..."; }
    public long getControlMaskBitFlags() { return 0x...; }
    }

    Snoofle, this sounds like a good candidate for a lookup table: a dictionary, keyed off of "n"; with a value consisting of a tuple with ruleNumber, ruleName, controlMaskBitFlags.

    Thusly elminating the thousands of tiny classes (unless the code requires all these subclasses to be floating around?)

    You are absolutely correct. The limiting factor here is that there is no way to specify map.get(n) in this rules engine; it spits back gibberish, so I was forced to create separate classes.

    I could have put the whole of the virtual lookup stuff into a matrix, db table, static structure, etc. This particular mechanism was requested by the person who's going to be responsible for this going forward. I figured I'll never see it again, and this is what they wanted so why not be nice about it?



  • @boomzilla said:

    Filed under: are there eels in your hovercrafts?

    // I wanted to write something about my nipples being aligned with something, but it wasn't funny so I wrote this comment instead.



  • @snoofle said:

    One aspect of our system is rule driven.

    Do you mean aspect, or [url=http://en.wikipedia.org/wiki/Aspect-oriented_programming]aspect[/url]?



  • @TheCPUWizard said:

    Software is not done until the last running instance has been removed from production, and it is demonstrably proven that the data will never be needed again!

     

     

     

    ... or when no one has the budget to pay for any more work on it.

     



  • @TheCPUWizard said:

    @dhromed said:

    @Ibix said:

    not think "there has to be a better way"
     

    Maybe he did, but thought git 'er done had the greater priority. And then there was the next project. That's how it goes.

    was asked to make a change to one of the rules.

    Software is not done until the last running instance has been removed from production, and it is demonstrably proven that the data will never be needed again!

    Lip lop fizzle fazzle hoo hoo haa haa
    Tip top froo froo flim flam hizzle huzzle

    Limp dick shizzle shizzle moomoo maamaa
    Ape ape haww haww thuc pham a dong

    Poo poo boo boo tuc tuc yuckel yuckel
    Marmalade yarmulke pineapple express

    Hing hing tickle tickle moo goo moo shu
    Flip flop giggle giggle help me get long



  • @bridget99 said:

    Lip lop fizzle fazzle hoo hoo haa haa Tip top froo froo flim flam hizzle huzzle Limp dick shizzle shizzle moomoo maamaa Ape ape haww haww thuc pham a dong Poo poo boo boo tuc tuc yuckel yuckel Marmalade yarmulke pineapple express Hing hing tickle tickle moo goo moo shu Flip flop giggle giggle help me get long

    The sad thing is this makes more sense than bridget99's normal posts.


Log in to reply