Hacker News new | ask | show | jobs
by sequoia 4634 days ago
warning: typical HN nitpicking (I know, typical, but it was just my first reaction upon seeing the code & I want to be honest).

I looked at the code and my first impression wasn't good. These are minor issues but such "smells" are red flags (to mix metaphors) and warn me away from the codebase

https://github.com/circa75/dropplets/blob/master/index.php#L...

    } else if($_GET['filename'] == 'rss' || $_GET['filename'] == 'atom') {
why would you set a get var called "filename" to request an RSS feed? There is no actual file called `rss`, so it's not a file name. So the filename get var is really maybe a file name, maybe some other non-file resource being requested. This is confusing & unintuitive, makes the code harder to read & parse.

https://github.com/circa75/dropplets/blob/master/index.php#L...

    $posts = ($pagination_on_off != "off") ? array_slice($all_posts,$offset,($posts_per_page > 0) ? $posts_per_page : null) : $all_posts;
too many ternaries!! Why are you testing against a specific string ("false") to indicate a boolean value? This is a major smell as it suggests the author doesn't know the proper use of basic PHP data types.* Why are you adding a ternary if to test posts_per_page size rather than just setting it to `null` to indicate "all posts" in the first place? How can pagination be true with unlimited posts per page anyway? Make posts_per_page null by default, remove redundancy in pagination config var name, make it a bool, and you're left with:

    $posts = $paginate ? array_slice($all_posts, $offset, $posts_per_page) : $all_posts;
I don't mean to condemn the efforts of the author: everyone starts somewhere and this looks like a pretty dang cool project, but smells like these make me walk away immediately because they indicate this code is going to be confusing and hard to work with. One last one: functions.php. This will send a cold chill down the spine of any PHP dev familiar with the bad old days. Group code logically, write reusable modules, and use namespaces (well skip this last one if you want to support old/cheap hosting)! No need to code like its PHP 4 anymore :)

* Yes I see that it's a user defined setting in settings.php or config.php and an argument could be made that a user would understand "on"/"off" more easily than true/false, but this is a trivially solved by a comment & would make the code more robust and readable. More robust because string comparison in PHP is case sensitive, so if the user sets the setting to "OFF" it will effectively be "on." Boolean solves this.

1 comments

I'm actually the one that wrote the last two nasty parts of code, not the author himself. I'm really sorry that it confused you.

I actually don't remember why I wrote them like that, but I'm pretty sure I had a reason (at least I think I do..)

Again, sorry about that.

And this is how PHP hell begins.
Not unique to PHP in any way shape or form.
Perhaps not, but PHP certainly makes it unusually easy.
I know this is a popular idea, but it's pretty easy to write terrible code in any language. See: most code.
Yes, but, in most langauges, this doesn't work

    <?php
    $a = 0;
    $b = 'x';
    var_dump(FALSE == $a);
    var_dump($a == $b);
    var_dump($b == TRUE);
    ?>
This results in

    bool(true)
    bool(true)
    bool(true)
Please elaborate. :(
You wrote nasty code in PHP that you don't clearly understand, and aren't immediately either researching (and then documenting!) why you did what you did, or failing that, recoding for sanity. Once those little hacks calcify it's all down the tubes from there.
And this is how HN threads get derailed and ruined. Your comment is valueless, please consider deleting it.