HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

I CAN HAS EULR 14, PLZ?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
hascaneulrplz

Problem

A few days ago I found myself learning lolcode. It turns out it's (almost) a real language and capable of handling (pseudo-)serious problems. So, let's try some (pseudo-)serious code this time.

Project Euler 14

A Collatz sequence is defined for the set of positive integers:

n → n/2 (n is even)
n → 3n + 1 (n is odd)


Using this rule, a sequence can be generated.

Which starting number, under one million, produces the longest chain?

OBTW
WHICH STARTIN NUMBR, UNDR WAN MILLION, PRODUCEZ TEH LONGEST CHAIN?
TLDR

HAI 1.3

HOW IZ I collatz YR currentNumber
I HAS A cowntr ITZ 1
IM IN YR collatzLoop
BOTH SAEM 0 AN MOD OF currentNumber AN 2, O RLY?
YA RLY, currentNumber R QUOSHUNT OF currentNumber AN 2
NO WAI, currentNumber R SUM OF PRODUKT OF currentNumber AN 3 AN 1
OIC
cowntr R SUM OF cowntr AN 1
BOTH SAEM currentNumber AN 1, O RLY?
YA RLY, FOUND YR cowntr, OIC
IM OUTTA YR collatzLoop
IF U SAY SO

HOW IZ I longestCollatz YR maxNumber
VISIBLE maxNumber
I HAS A maxLengthFound ITZ 0
I HAS A currentLength ITZ 0
I HAS A longestAtValue ITZ 0
I HAS A cowntr ITZ 1
IM IN YR maxCollatzLoop
currentLength R I IZ collatz YR cowntr MKAY
DIFFRINT currentLength AN SMALLR OF currentLength AN maxLengthFound, O RLY?
YA RLY, maxLengthFound R currentLength, longestAtValue R cowntr, OIC
cowntr R SUM OF cowntr AN 1
BOTH SAEM cowntr AN maxNumber, O RLY?
YA RLY, FOUND YR longestAtValue, OIC
IM OUTTA YR maxCollatzLoop
IF U SAY SO

VISIBLE I IZ longestCollatz YR 1000000 MKAY

KTHXBYE


Took only 3 minutes to execute on my machine, lightning fast!

Of-course, in any real language with a real implementation, this should take under 5 seconds. But lolcode isn't very fast and it's lacking a lot of performance constructs. For example, usually you'd memoization or recursion. I'm pretty sure the latter can't be done and the former would get very ugly very fast.

Solution

Indentation

BOTH SAEM currentNumber AN 1, O RLY?
  YA RLY, FOUND YR cowntr, OIC


You're missing indentation here. In more mainstream languages, this construct looks like so:

if (currentNumber == 1)
  return cowntr;


This is hard to read, and would be better if it were indented:

if (currentNumber == 1)
      return cowntr;


BOTH SAEM currentNumber AN 1, O RLY?
      YA RLY, FOUND YR cowntr
  OIC


Plus it's a good idea to just use braces in the first place.

The same thing goes for the constructs later on:

DIFFRINT currentLength AN SMALLR OF currentLength AN maxLengthFound, O RLY?
  YA RLY, maxLengthFound R currentLength, longestAtValue R cowntr, OIC


BOTH SAEM cowntr AN maxNumber, O RLY?
  YA RLY, FOUND YR longestAtValue, OIC


These also need proper indentation.

If you don't want to use the brace style, then just indent the YA RLY case one level. That way you get the indented look of mainstream languages...

if (condition)
    do thing


... which is easier to quickly parse than a wall of left-aligned code.

Bugs

Collatz of 1 doesn't return 1. It returns 3.

Loops

Looking at your code some more, I get the feeling that you went out of your way to dodge the regular loop structure as it didn't do what you wanted it to do the last time. But as a result you have to write your own incrementer and your own exit condition.

I think you'd be better off by using the regular loop structure. You can rely on the return.

HOW IZ I collatz YR currentNumber
  I HAS A cowntr ITZ 1
  IM IN YR collatzLoop
    BOTH SAEM 0 AN MOD OF currentNumber AN 2, O RLY?
      YA RLY, currentNumber R QUOSHUNT OF currentNumber AN 2
      NO WAI, currentNumber R SUM OF PRODUKT OF currentNumber AN 3 AN 1
    OIC
    cowntr R SUM OF cowntr AN 1
    BOTH SAEM currentNumber AN 1, O RLY?
    YA RLY, FOUND YR cowntr, OIC
  IM OUTTA YR collatzLoop
IF U SAY SO


becomes

HOW IZ I collatz YR currentNumber
  IM IN YR collatzLoop UPPIN YR cowntr
    BOTH SAEM currentNumber AN 1, O RLY?
    YA RLY, 
      FOUND YR SUM OF cowntr AN 1
    OIC
    BOTH SAEM 0 AN MOD OF currentNumber AN 2, O RLY?
      YA RLY, currentNumber R QUOSHUNT OF currentNumber AN 2
      NO WAI, currentNumber R SUM OF PRODUKT OF currentNumber AN 3 AN 1
    OIC
  IM OUTTA YR collatzLoop
IF U SAY SO


And this loop...

I HAS A cowntr ITZ 1
IM IN YR maxCollatzLoop
  currentLength R I IZ collatz YR cowntr MKAY
  DIFFRINT currentLength AN SMALLR OF currentLength AN maxLengthFound, O RLY?
  YA RLY, maxLengthFound R currentLength, longestAtValue R cowntr, OIC
  cowntr R SUM OF cowntr AN 1
  BOTH SAEM cowntr AN maxNumber, O RLY?
  YA RLY, FOUND YR longestAtValue, OIC
IM OUTTA YR maxCollatzLoop


Becomes

IM IN YR maxCollatzLoop UPPIN YR cowntr WILE BOTH SAEM maxNumber AN BIGGR OF cowntr AN maxNumber
  I HAS A collatzNumbar ITZ SUM OF cowntr AN 1
  currentLength R I IZ collatz YR collatzNumbar MKAY
  DIFFRINT currentLength AN SMALLR OF currentLength AN maxLengthFound, O RLY?
  YA RLY, 
    maxLengthFound R currentLength
    longestAtValue R collatzNumbar 
  OIC
IM OUTTA YR maxCollatzLoop
FOUND YR longestAtValue


The reason my versions are longer is because I opted to split your multiple statements to individual lines.

Version number

OBTW
    WHICH STARTIN NUMBR, UNDR WAN MILLION, PRODUCEZ TEH LONGEST CHAIN?
TLDR

HAI 1.3


I think I said something about this before, but I'll say it again; version number goes at the top. It's needed for the interpreter. Even if they work fine without the version number at the top, I think it'd be best to put the version number at the top so that interpreters don't have to go searching for it.

Code Snippets

BOTH SAEM currentNumber AN 1, O RLY?
  YA RLY, FOUND YR cowntr, OIC
if (currentNumber == 1)
  return cowntr;
if (currentNumber == 1)
      return cowntr;
BOTH SAEM currentNumber AN 1, O RLY?
      YA RLY, FOUND YR cowntr
  OIC
DIFFRINT currentLength AN SMALLR OF currentLength AN maxLengthFound, O RLY?
  YA RLY, maxLengthFound R currentLength, longestAtValue R cowntr, OIC

Context

StackExchange Code Review Q#135069, answer score: 4

Revisions (0)

No revisions yet.