• theunknownmuncher@lemmy.world
    link
    fedilink
    arrow-up
    79
    ·
    edit-2
    11 days ago

    Whenever you sit back and smile proudly to yourself about how clever the block of code you just wrote is, your next move should be to delete and rewrite it.

    This is a clever block of code! Great job, now rewrite it to be sane 😂

    • balsoft@lemmy.ml
      link
      fedilink
      arrow-up
      25
      ·
      11 days ago

      I think it depends; some smart code is good actually, think 0x5f3759df. As long as you properly document it and leave plenty of comments. This one is not smart though, at best it’s what I would call witty.

  • CookieOfFortune@lemmy.world
    link
    fedilink
    arrow-up
    43
    ·
    11 days ago

    This isn’t sufficiently enterprisey for Java. There should be a Roman numeral factory followed by relevant fromString and toInteger methods.

    • vithigar@lemmy.ca
      link
      fedilink
      arrow-up
      7
      ·
      10 days ago

      Ugh. Literally refactored multiple factories into straightforward functions in the most recent sprint where I work.

      Someone saw a public factory method which was a factory for a reason and just cargo culted multiple private methods using the same pattern.

  • TootSweet@lemmy.world
    link
    fedilink
    English
    arrow-up
    23
    ·
    edit-2
    11 days ago

    My first thought was something along the lines of a “zip bomb”. For every “M” in the input string, it’d use more than a KiB of memory. But still, it’d take a string of millions of "M"s to exhaust memory on even a low-end modern server. Still probably not a good idea to expose to untrusted input on a public networked server, though. And it could easily peg a CPU core for a good while. Very good leveraged target for DDOSing.

  • rooroo@feddit.org
    link
    fedilink
    arrow-up
    16
    ·
    11 days ago

    It also works the other way round: wanna convert Arabic n to Roman? Just write n times ‘I’ and revert these replacement in inverse order.

    • lugal@lemmy.dbzer0.com
      link
      fedilink
      arrow-up
      5
      ·
      11 days ago

      I don’t know what happens when the substring overlaps. Like for the number 6, will it replace the first 5 I’s with V and end up correctly with VI or the last ones and come to IV? I would guess the former and maybe you know but I never thought about it before

      • rooroo@feddit.org
        link
        fedilink
        arrow-up
        2
        ·
        10 days ago

        I’ve written it that way and it works, as in it will replace left to right and you replace iiii to iv after iiiii to v

        • lugal@lemmy.dbzer0.com
          link
          fedilink
          arrow-up
          0
          ·
          10 days ago

          Makes sense but it will fail at 9 (VIV) it would only work for 9 if the replace went from right to left or the V and IV statements were exchanged but in both cases, 6 would fail

          • rooroo@feddit.org
            link
            fedilink
            arrow-up
            1
            ·
            9 days ago

            9 is IX though, and that works.

            6 works fine, as it replaces the first set of 5 I with V and then there’s nothing to replace.

            I’d written it in typescript for all it’s worth; go ahead and try it yourself :)

              • rooroo@feddit.org
                link
                fedilink
                arrow-up
                2
                ·
                9 days ago

                I like your questions about this and they all seem fair but I kinda wanna encourage you to go ahead and write it yourself; it’s a fun way to convert into Roman numerals that both is and isn’t intuitive at the same time.

              • rooroo@feddit.org
                link
                fedilink
                arrow-up
                2
                ·
                9 days ago

                No, cause you do the replacement from large to small, I.e. you’d first check for 10 I to replace with X (none found); then replace 9 with IX (check), then check for 5, 4 and so on.

                • lugal@lemmy.dbzer0.com
                  link
                  fedilink
                  arrow-up
                  2
                  ·
                  9 days ago

                  The original doesn’t have an extra check for 9 and it works for Roman->Indioarabic because it’s:

                  IX
                  ->IVV
                  ->IIIIV
                  ->IIIIIIIII
                  

                  But the other way around, you need an extra step for 9. That’s where our misunderstanding comes from.

    • trxxruraxvr@lemmy.world
      link
      fedilink
      arrow-up
      35
      ·
      edit-2
      11 days ago

      No, M will be replaced by DD and then CD will be picked up, so it will go

      1. CM
      2. CDD
      3. CCCCD
      4. CCCCCCCCC
  • Caveman@lemmy.world
    link
    fedilink
    arrow-up
    6
    ·
    10 days ago

    It’s not too bad, it’s readable and easily optimised by adding intermediate sums and removing whatever power of 10 you’re working on.

    • grue@lemmy.world
      link
      fedilink
      arrow-up
      17
      ·
      edit-2
      11 days ago

      Code gulf, you say?

      public static String
      convertRomanNumeral(String numeral) {
          numeral = numeral.replace("America", "Mexico");
          return numeral;
      } 
      
    • ray@sh.itjust.works
      link
      fedilink
      arrow-up
      2
      ·
      11 days ago
      public static int convertRomanNumeral(String numeral)
      {
        numeral = numeral.replace("M", "DD")
          .replace("CD", "CCCC")
          .replace("D", "CCCCC")
          .replace("C", "LL")
          .replace("XL", "XXXX")
          .replace("L", "XXXXX")
          .replace("X", "VV")
          .replace("IV", "IIII")
          .replace("V", "IIIII");
        return numeral.length();
      }
      
      • qaz@lemmy.worldOP
        link
        fedilink
        English
        arrow-up
        4
        ·
        10 days ago
        public static int convertRomanNumeral(String numeral)
        {
          return numeral.replace("M", "DD")
            .replace("CD", "CCCC")
            .replace("D", "CCCCC")
            .replace("C", "LL")
            .replace("XL", "XXXX")
            .replace("L", "XXXXX")
            .replace("X", "VV")
            .replace("IV", "IIII")
            .replace("V", "IIIII")
            .length();
        }