Howdy, Stranger!

It looks like you're new here. If you want to get involved, click one of these buttons!

Try Vanilla Forums Cloud product

The Hall Of Code Curiosities

mtschirsmtschirs ✭✭✭
edited August 2015 in Development

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

-

peregrineAaronWebstey
«1

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???
    

    -

    peregrineAaronWebstey
  • 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.

    My themes: pure | minusbaseline - My plugins: CSSedit | HTMLedit | InfiniteScroll | BirthdayModule | [all] - PM me about customizations

    peregrineAaronWebstey
  • LincLinc Vanilla's Bard (and Director of Development) Detroit Vanilla Staff

    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.

    R_JAaronWebstey
  • 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.

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

    -

    LincGolamy
  • LincLinc Vanilla's Bard (and Director of Development) Detroit Vanilla Staff

    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.

    mtschirsBleistivtGolamy
  • 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:

    -

    peregrineGolamyAaronWebstey
  • 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:

    -

    peregrine
  • 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:

    -

    AaronWebstey
  • 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.

    hgtonight
  • 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."
    );
    

    -

    R_Jperegrine
  • LincLinc Vanilla's Bard (and Director of Development) Detroit Vanilla Staff

    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')) ...
    

    -

    BleistivtAaronWebsteyhgtonightvrijvlinder
  • @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

    My themes: pure | minusbaseline - My plugins: CSSedit | HTMLedit | InfiniteScroll | BirthdayModule | [all] - PM me about customizations

    mtschirsAaronWebsteyhgtonightvrijvlinder
  • mtschirsmtschirs ✭✭✭
    edited September 2015

    @Bleistivt I read your link, but it talks about data types.

    My point (and the topic of the poll) was that valr() & co. are not recursive functions :)

    If one would want to argue that valr() is indeed recursive since it makes use of arrays which are recursively defined data types, then every function using an array would have to be called recursive. If the argument is that valr() traverses a recursive data type, then every for-loop over an array would have to be called recursive, too :winky:

    -

  • hgtonighthgtonight ∞ · New Moderator

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

    n < 20 :anguished:

    Vanilla is probably bipolar. Massaging one line, refusing to hold hands the next day:

    https://github.com/vanilla/vanilla/blob/1ae06b/applications/dashboard/controllers/class.entrycontroller.php#L1008

    @mtschirs said:
    Bleistivt I read your link, but it talks about data types.

    My point (and the topic of the poll) was that valr() & co. are not recursive functions :)

    There was never any question whether the function was iterative. The name refers to the data type it works with.

    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.

    AaronWebsteymtschirs
  • mtschirsmtschirs ✭✭✭
    edited September 2015

    @hgtonight schrieb:
    n < 20 :anguished:

    you got me :unamused:

    @hgtonight schrieb:
    There was never any question whether the function was iterative. The name refers to the data type it works with.

    Let's consult our primary source here:

    if (!function_exists('getValueR')) {
        /**
         * Return the value from an associative array or an object.
         *
         * This function differs from GetValue() in that $Key can be a string consisting of dot notation that will be used
         * to recursively traverse the collection.
         ...
    

    So the postfix 'r' (whether it is a postfix or a suffix shall be the topic for a future debate) stands for "recursive traversal". Which is exactly what you, hgtonight, already suspected earlier. However, there is no recursive traversal. In fact, there can be no recursive traversal without recursive traversal function :expressionless:

    The argument that getValueR() performs recursive traversal because the variable it iterates through has a recursively-defined datatype could equally be used to argue that inspecting any array or object properties is recursive traversal - which is clearly not the case.

    This discussion shall remind all of us of the horrors that await those who prematurely name their functions.

    -

    vrijvlinderAaronWebsteyperegrine
  • R_JR_J Cheerleader & Troubleshooter Munich Moderator

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

    I'd say: 5 votes doesn't make a poll. But we could have a vote about that ;)

    mtschirshgtonightperegrine
  • KasperKasper Scholar of the Bits Copenhagen Vanilla Staff
    edited September 2015

    My 5 cents on the whole "recursive vs. iterative" discussion: The functions are by all means iterative and have nothing to do with recursion. The r-suffixed ones are however deep in the sense that they can traverse more than one level down object and array properties.

    Kasper Kronborg Isager (kasperisager) | Freelance Developer @Vanilla | Hit me up: Google Mail or Vanilla Mail | Find me on GitHub

    AaronWebsteyperegrine
  • In library/vendors - functions.webwizhash.php:

    // This code is for private use only
    

    I hope nobody uses Vanilla commercially :glasses:

    -

    peregrine
  • LincLinc Vanilla's Bard (and Director of Development) Detroit Vanilla Staff
    edited September 2015

    @mtschirs said:
    In library/vendors - functions.webwizhash.php:

    // This code is for private use only
    

    I hope nobody uses Vanilla commercially :glasses:

    Now you're referencing code that isn't even ours :silenced:

  • mtschirsmtschirs ✭✭✭
    edited September 2015

    @Linc schrieb:
    Now you're referencing code that isn't even ours :silenced:

    That's kind of the point. Vanilla includes "private use only" code that some of us (and you) use commercially. Not that I care, but the author of that library might :tongue:

    -

    peregrine
  • LincLinc Vanilla's Bard (and Director of Development) Detroit Vanilla Staff

    @mtschirs said:
    That's kind of the point. Vanilla includes "private use only" code that some of us (and you) use commercially.

    Given the context it is in, after this:

    Feel free to distribute as long as code is not modified, and header is kept intact

    and just before this:

    and the security and/or encryption of the resulting hexadecimal value is not warrented [sic] or gaurenteed [sic] in any way.

    I read that statement as part of a disclaimer, not a licensing restriction. The author is free to disagree or clarify.

    abdulsyukur
  • mtschirsmtschirs ✭✭✭
    edited September 2015

    The 3rd party license says:

    Feel free to distribute as long as code is not modified

    Todd ported the code from WebWiz's functions_hash1way.asp to Vanilla PHP. I would argue that a port is a modification.

    @Linc said:
    I read that statement as part of a disclaimer, not a licensing restriction. The author is free to disagree or clarify.

    I don't see much room for interpretation in "private use only". However, I see where you come from. That code is old, the author might not care and the whole thing is just an annoyance. But what if the author decides to care one day?

    Let's say you don't agree with me on these two points. Then here is why that code should really be removed from the Vanilla codebase:

    "Distribution or modification of programs, including incompatibly licensed code, will result in copyright infringement." ("Understanding Open Source and Free Software Licensing" by Andrew M. St. Laurent, O'Reilly Media, Inc., 2004)

    or

    "If I add a module to a GPL-covered program, do I have to use the GPL as the license for my module? The GPL says that the whole combined program has to be released under the GPL. So your module has to be available for use under the GPL." (gpl faq)

    Clearly, "Feel free to distribute as long as code is not modified" is not GPL compatible.

    -

    peregrinemcu_hqabdulsyukur
«1
Sign In or Register to comment.