Dropplets - A simple database-less CMS • Hacker News Board

Re: Dropplets - A simple database-less CMS

#51

Post by adambard » Mon, Oct 07, 2013, 6:04 PM UTC

It looks really good, but I hate a lot about the demo theme. Why does anything move without me doing anything? Will the annoying bottom bar always be there? What if I can't find a high-res artsy photo? Was it necessary to knock off Medium's (distinctive) menu icon?

Re: Dropplets - A simple database-less CMS

#52

Post by sequoia » Mon, Oct 07, 2013, 6:10 PM UTC

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.

Re: Dropplets - A simple database-less CMS

#53

Post by [deleted] » Mon, Oct 07, 2013, 6:22 PM UTC

sequoia wrote: post #52

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'] ==…

[deleted]

Re: Dropplets - A simple database-less CMS

#54

Post by bndr » Mon, Oct 07, 2013, 6:23 PM UTC

sequoia wrote: post #52

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'] ==…

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.

Re: Dropplets - A simple database-less CMS

#55

Post by TylerE » Mon, Oct 07, 2013, 6:31 PM UTC

bndr wrote: post #54

sequoia wrote: post #52

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'] ==…

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.

Re: Dropplets - A simple database-less CMS

#56

Post by bndr » Mon, Oct 07, 2013, 6:35 PM UTC

TylerE wrote: post #55

bndr wrote: post #54

Earlier quoted context omitted.

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.

Please elaborate. :(

Re: Dropplets - A simple database-less CMS

#57

Post by TylerE » Mon, Oct 07, 2013, 6:51 PM UTC

bndr wrote: post #56

TylerE wrote: post #55

Earlier quoted context omitted.

And this is how PHP hell begins.

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.

Re: Dropplets - A simple database-less CMS

#58

Post by ebbv » Mon, Oct 07, 2013, 6:59 PM UTC

TylerE wrote: post #55

bndr wrote: post #54

Earlier quoted context omitted.

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.

Re: Dropplets - A simple database-less CMS

#59

Post by TylerE » Mon, Oct 07, 2013, 6:59 PM UTC

ebbv wrote: post #58

TylerE wrote: post #55

Earlier quoted context omitted.

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.

Re: Dropplets - A simple database-less CMS

#60

Post by sterlingross » Mon, Oct 07, 2013, 7:58 PM UTC

reidrac wrote: post #11

"Dropplets is compatible with most server configurations [...]". Requires PHP (at least), I guess none of my servers has a common configuration :) EDIT: the installation part in the README.md needs some extra info. Like it requires PHP and some file/directory permissions.

In case anyone is curious, it seems you need to add full write access to the root install directory as the config.php file is added there.