Refactoring

[elrido]

TL;DR: after adding many features, a bit of cleanup is needed IMO

After having added quite some additional features in 0.20 and 0.21 mainly zerobin.php starts to ballon a little and I started to get quite a few bugs in the logic recently (thanks @Hexalyse for finding some of them).

Lots of the code in there is about validation before data manipulation. If we think of zerobin as a controller, our example is getting a bit to fat to maintain. Also these issues slipped my unit tests, as lots of this logic can only be tested by running the full app (case in point: tst/zerobin.php is 7 times as large as the next largest unit test). If the logic would be more modular, it would be easier to test and maintain and therefore safer and more bug free.

Suggested refactoring targets:

• providing a configuration object. It could contain safe default values, so when an admin accidentally misses an option in the config file (or the config file is not available or empty) the app still works and we do not need to know the default value when accessing something in the configuration.
• data models. Currently the main class configures the data access model, validates incoming data and directly stores and retrieves raw data in these. Instead we could have objects for pastes and comments and these would be given the configuration object on creation and would handle the data validation.
• router. Generally we could hand off the "routing" into a separate class. The router would decide a) if we are in a JSON API (see below) or an HTML context and b) what type of operation (CRUD) we want to perform.
• provide a clean JSON REST API. Responses should be JSON instead of HTTP HTML, when the HTTP header "X-Requested-With: JSONHttpRequest" is sent. The default of our current jQuery calls is "X-Requested-With: XMLHttpRequest", but this can be changed. The operations would be:
  • create paste - POST data to /index.php, return status with ID of new paste
  • create paste - PUT data to /index.php?[ID of new paste], return status
  • delete paste - DELETE data: {'secret': '[deletion token]'} to /index.php?[ID of paste to delete], return status
  • read paste - GET to /index.php?[ID of paste], return status containing paste data
  • create comment - POST data to /index.php?[ID of paste]&[parent comment ID], return status with ID of new comment
  • create comment - PUT data to /index.php?[ID of paste]&[parent comment ID]&[comment ID], return status
  • list all comments - GET to /index.php?[ID of paste], status with comment list
  • list child comments (only direct children) - GET to /index.php?[ID of paste]&[parent comment ID], status with comment list
  • read comment - GET to /index.php?[ID of paste]&[parent comment ID]&[comment ID], return status containing comment data
  • note the lack of a "delete comment" operation, comments are deleted with their parent pastes

I am not yet quite so sure what to do about the JS part. Maybe it would be an option to start to look into angular? In any case I'd like to look into things like JSUnit und JSDocs both to avoid issues in there and help us get an overview of it.

[Hexalyse]

Nice initiative !

About AngularJS, I'm currently using this technology at work, and I'm wondering: wouldn't it be a bit "overkill" for this App ? Considering the original aim of ZeroBin was to be "minimalistic", using AngularJS wouldn't really be minimalistic, since it's a big framework.

[elrido]

Yeah, using Angular in the company, too. Its quite rich and usually you use it with build tools (i.e. gulp or similar). I can see it becoming an overkill for the intended use case of ZeroBin. Maybe it could be done as an alternative client at a later point.

I see that Vikstrous had started to use the Backbone.js library in his fork. This seems more lightweight, but I have no experience with it.

I am mainly looking for the right approach to keep zerobin.js easy to maintain. Maybe some JS guru has some suggestions? Will focus on the php part for now.

[elrido]

I made some progress on this during the weekend. Found and solved a few issues that I had not yet discovered. Implemented a "request" instead of a "router", routing is still done in the main class, but is now only a few lines. All in all, zerobin.php shrank by 200+ lines or over a third. Of course the additional classes more then make up for that, but it should now be a lot easier to maintain.

To my surprise I discovered that DELETE and PUT are not available on all webservers by default. Now I can work around that on my system, but many shared hosting users may not. So for now we will have to work with GET for DELETEs and POST instead of PUT to stay compatible among all installations.

The API part is already partially implemented, but I will have to extend the data classes a bit to allow more targeted access on the comments, to be able to support all of the above listed operations.

[elrido]

The main refactoring points and most API changes are complete. The only missing operations are "list child comments" and "read single comment". These are (currently) not required.

The JSON-LD-notation and -context was implemented is to make it possible ZeroBin installs on different versions can discover what features are supported by another install i.e. for scenarios as described in #33. For the same reason CORS headers were implemented. The comment_count and comment_offset were added in preparation of #44.

[elrido]

As the PHP and API part of the issue were resolved, but no input on the JS part was received, I am closing the issue for now.