The Hall Of Code Curiosities

mtschirs

I just stumbled over this code (simplified for presentation purposes):

$CssClass = 'Button Action Big Primary';
$Css = 'Button Primary Action NewPoll';
$Css .= strpos($CssClass, 'Big') !== FALSE ? ' BigButton' : '';
echo ButtonGroup($this->Buttons, $Css, $this->DefaultButton);

This summarizes my experience with the Vanilla codebase today quite well .

Let us collect more such snippets! This lighthearted thread shall be the much needed place to vent our experiences with the code we love .

mtschirs

Found some quite emotional TODOs

// TODO rm global
// TODO nope
// TODO global nope x2
// TODO wtf globals

// TODO: DO I NEED THIS???
// TODO: DO I REALLY NEED THIS???

Bleistivt

To be fair, the first one actually gives you a little more control.

Between the first and the second line in your abbreviated example, the BeforeNewDiscussionButton event is fired, where you could modify $CssClass to not contain "Big". But yes, it is more complicated than it needs to be.

Linc

Sometimes weirdness like that creeps in to support a weird client requirement. If you're paying us enough money a month, we'll sully our code a little for you. A little.

hgtonight

Mind linking to the code blocks?

mtschirs

On second thought, let's rename this thread into something more positive! I intended to poke a bit of fun, not blame anyone. If none of you native speakers comes up with a better name I suggest "Coding Curiosities"

Linc

Seems like a fine place to add: We're big fans of humorous, informative code comments. It's helpful to remember the old "Bonk: something funky happened!" error page to put yourself in the right frame of mind to code in Vanilla's voice. Exasperation is ok, angry is not.

mtschirs

Gdn_Request::isPostBack() returns true for POST requests...
Gdn_Form::isPostBack() returns true for POST ...and GET requests...
Gdn_Form::isMyPostBack() does exactly the same, but states in its comment that is does not.

peregrine

@mschirs

hope your suggestions are taken too heart rather than viewed only as humor.

mtschirs

You would think that Gdn_Form::setValue() and Gdn_Form::getValue() are complimentary... and you would be right, unless somebody adds, lets say, a "?lol" to your site's URL. Then, you have to use getFormValue() to retrieve values set via setValue(), or setData(), that is, or setFormValue(), but only in this case, not if the visitor chose to omit the "?lol" or the "?gotohell" or "?youmad" or whatever he chose to add to ruin your day.

mtschirs

What do these functions have in common?
getValueR() valr() svalr() setvalr()

The r suffix! What does the r suffix stand for? Recursive! Are these functions recursive? No!

hgtonight

@mtschirs said:
What do these functions have in common?
getValueR() valr() svalr() setvalr()

The r suffix! What does the r suffix stand for? Recursive! Are these functions recursive? No!

Recursive refers to what it does, not how it accomplishes it. You can recurse into an object without using a recursive function, no?

mtschirs

No. What would the re in your recurse stand for? There is no recursion. You could unroll a recursion using a stack and if it is a tail-recursive function you only need to store the last state instead of the whole stack. None of these mechanisms is at work here.

hgtonight

The function is traversing recursively through the object via an iterative for loop.

valr for example:

 function valr($key, $collection, $default = false) {
    $path = explode('.', $key);

    $value = $collection;
    for ($i = 0; $i < count($path); ++$i) {
       $subKey = $path[$i];

       if (is_array($value) && isset($value[$subKey])) {
          $value = $value[$subKey];
       } elseif (is_object($value) && isset($value->$subKey)) {
          $value = $value->$subKey;
       } else {
          return $default;
       }
    }
    return $value;
 }

The traversal is recursive since each loop is changing the value. The implementation isn't recursive, but the traversal is recursive.

On a side note, it seems silly to me to name a function after the implementation versus what it does. Unless you are directly comparing implementations for testing? idk

mtschirs

Well, to me it is clearly iterative, yes, but not recursive. To my knowledge, 'recursive traversal' applies only to those cases where a recursive path or tree traversal function is used.

Now that the positions are clear - recursive vs. iterative - we need an audience vote http://www.poll-maker.com/poll396839xB56a481c-15

Bleistivt

https://en.wikipedia.org/wiki/Recursive_data_type

AaronWebstey

I think this is my favourite thread of all time.

mtschirs

Lies, damned lies, and statistics...

if ($CountDownloads < 500000) $CountDownloads = 500000;
$this->SetData(
    'Description',
    "Vanilla is forum software that powers discussions on $CountDownloads sites. Built for flexibility and integration, Vanilla is the best, most powerful community solution in the world."
);

Linc

I've run into people equating downloads with sites more than once. The if statement is either a fallback or from a stats migration. I know the current community codebase was written after Vanilla had more than that many downloads.

mtschirs

We finally have a result on our iterative vs recursive poll - and the winner is... iterative

Also, this (from library/../FirePHP.php):

// oh please oh please oh please oh please oh please
if (function_exists('mb_convert_encoding')) ...

Bleistivt

@mtschirs said:
We finally have a result on our iterative vs recursive poll - and the winner is... iterative

but one is for recursive data types while the other isn't why aren't you all reading wikipedia

Also I can't remember vanilla ever massaging me
https://github.com/vanilla/vanilla/blob/1ae06b/applications/dashboard/controllers/class.entrycontroller.php#L578