Some things should stay hidden



  • One of the sites we maintain is written in JSP and carries a JAR file containing a few utility classes used throughout the site. Today I decided that it was time to add an extra function to one of these classes and so I downloaded a java decompiler to peak inside the JAR file. Sadly the original source files are long gone so we will never know if there were any comments. If the original  programmer had bothered with them however, they would have undoubtedly provided us with an extra source of amusement/horror. In any case I've decided to add a couple of my own comments, if for no other reason than to ease the pain.
    Still you'll need the heavy duty goggles for this one.

    //Function to convert newline characters to <br>
    //startup - initial text
    //replace - whether we actually want to replace the characters
    //keep - Number of characters of the new result that we want to keep
    //howmany - How many <br> strings to replace each newline with
      public String nToBr(String startup, boolean replace, int keep, int howmany)
      {
        String result = "";   
        if (replace) {    //Only do what the function's supposed to do if this is true
          startup = startup.trim();
          int i = -1;
          int len = startup.length();
          char[] chr = new char[1];
          char[] chr2brs = new char[8];
          chr2brs[0] = '<';
          chr2brs[1] = 'b';
          chr2brs[2] = 'r';
          chr2brs[3] = '>';
          chr2brs[4] = '<';
          chr2brs[5] = 'b';
          chr2brs[6] = 'r';
          chr2brs[7] = '>';
          char[] chr1br = new char[4];
          chr1br[0] = '<';
          chr1br[1] = 'b';
          chr1br[2] = 'r';
          chr1br[3] = '>';
          char[] chars = new char[startup.length()];
          chars = startup.toCharArray();
          while (++i < len)   
            if (chars[i] != '\n') {
              chr[0] = chars[i];
              result = result.concat(new String(chr));
            } else {
              if (howmany == 1)
                result = result.concat(new String(chr1br));

              if (howmany == 2)
                result = result.concat(new String(chr2brs));
            }
        }
        else
        {
          result = startup;
        }
        if (keep > 0) //Only return part of the initial string
        {
          for (int j = keep; j < result.length(); ++j)
            if ((result.charAt(j) == ' ') || (result.charAt(j) == '<'))
              break;

          if (j > result.length()) {    //If we're asked to keep more than the length of the original string, just.. err... assign the string to itself
            j = result.length();
            result = result.substring(0, j);
          } else {
            result = String.valueOf(result.substring(0, j)).concat(String.valueOf(" ..."));
          }

        }
        return result;
      }



  • So the DECOMPILED code looks like that? I'm not shocked at all. TRWTF is that you had to decompile it in the first place, and that you where expecting sane code.



  • @Daid said:

    So the DECOMPILED code looks like that? I'm not shocked at all. TRWTF is that you had to decompile it in the first place, and that you where expecting sane code.
     

    I'm pretty sure the decompiled version of a correcly written function would look something similar to:


    public String nToBr(String original){
        return original.replaceAll('\n', '<br/>');
    }

    But that's just me.


  • BINNED

    But if you look at the input parameters that's not what this function does. And even if were, the ridiculous string initilization probably is a result of the decompilation.

    It is needlessly complicated though and the hardcoded "howmany==1" and "howmany==2" is quite stupid.

     



  • I don't really know Java, but here is a C#-ish function that should do the same thing (properly):

    public string nToBr(string startup, bool replace, int keep, int howmany){
    string result=startup;
    if(replace){
    result=result.Trim();
    string brs="";
    for(int i=0; i<howmany; i++){
    brs+="<br>";
    }
    result.replace("\n",brs);
    }
    if(keep>0){
    int splitpoint=result.IndexOfAny(" <".ToCharArray(),keep);
    if(splitpoint==-1) splitpoint=result.Length();
    if(startup.Length> splitpoint) result=result.SubString(0, splitpoint) + "...";
    }
    return result;
    }

    All in all, a rather odd function, implemented badly.



  • Sure, the char array initialisation is just the decompiler being too lazy, but the quadratic time string copy (look at how chr is used) is original programmer WTF. And as for the utterly bizarre howmany handling, the less said, the better.


Log in to reply