HackerOne users: Testing against this community violates our program's Terms of Service and will result in your bounty being denied.

The Hall Of Code Curiosities

mtschirsmtschirs ✭✭✭

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 :grin:.

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

Tagged:
«13

Comments

  • mtschirsmtschirs ✭✭✭

    Found some quite emotional TODOs :grin:

    // TODO rm global
    // TODO nope
    // TODO global nope x2
    // TODO wtf globals
    
    // TODO: DO I NEED THIS???
    // TODO: DO I REALLY NEED THIS???
    
  • 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.

  • LincLinc Detroit Admin

    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.

  • hgtonighthgtonight ∞ · New Moderator

    Mind linking to the code blocks?

    Search first

    Check out the Documentation! We are always looking for new content and pull requests.

    Click on insightful, awesome, and funny reactions to thank community volunteers for their valuable posts.

  • mtschirsmtschirs ✭✭✭

    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" ;)

  • LincLinc Detroit Admin

    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.

  • mtschirsmtschirs ✭✭✭
    edited August 2015

    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.

    :dizzy:

  • peregrineperegrine MVP
    edited August 2015

    @mschirs

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

    I may not provide the completed solution you might desire, but I do try to provide honest suggestions to help you solve your issue.

  • mtschirsmtschirs ✭✭✭
    edited August 2015

    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.

    :angry:

  • mtschirsmtschirs ✭✭✭
    edited August 2015

    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! :tongue:

  • hgtonighthgtonight ∞ · New Moderator

    @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! :tongue:

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

    Search first

    Check out the Documentation! We are always looking for new content and pull requests.

    Click on insightful, awesome, and funny reactions to thank community volunteers for their valuable posts.

  • mtschirsmtschirs ✭✭✭
    edited August 2015

    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.

  • hgtonighthgtonight ∞ · New Moderator

    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

    Search first

    Check out the Documentation! We are always looking for new content and pull requests.

    Click on insightful, awesome, and funny reactions to thank community volunteers for their valuable posts.

  • mtschirsmtschirs ✭✭✭

    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 :hurrah:http://www.poll-maker.com/poll396839xB56a481c-15

  • AaronWebsteyAaronWebstey Headband Afficionado Cole Harbour, NS ✭✭✭

    I think this is my favourite thread of all time.

  • mtschirsmtschirs ✭✭✭
    edited August 2015

    Lies, damned lies, and statistics... :p

    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."
    );
    
  • LincLinc Detroit Admin

    I've run into people equating downloads with sites more than once. :angry: 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.

  • mtschirsmtschirs ✭✭✭
    edited September 2015

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

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

    // oh please oh please oh please oh please oh please
    if (function_exists('mb_convert_encoding')) ...
    
  • @mtschirs said:
    We finally have a result on our iterative vs recursive poll - and the winner is... iterative :chuffed:

    :'( but one is for recursive data types while the other isn't :'( why aren't you all reading wikipedia :unamused::anguished::scream::rage::heartbreak:

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

Sign In or Register to comment.