GHOP #74: Write a wrapper module for the Google Chart API

subscribe.

Hi,
That's my first draft. It has lasted so long due to reasons independent to me.
In source file there is draft HowTo with explanation of API, whatsoever there is an example at the bottom (in commentary). My code supports about 75% of GChart features, feel free to post all opinions of yours.

Marking for review.

The student said this is just a draft, and not finished, but I'll keep this at CNR for now so that people can take a look and see if it's headed in the right direction.

One thing I noticed from a brief scan of the code--it's not conforming to the Drupal coding standards (http://drupal.org/coding-standards). Specifically, there are whitespace issues and control structures aren't coded correctly (braces are in the wrong places).

Unfortunately I have been working on a VERY similar project, aka Google Charting API as well. I suppose now we will simply see which will come out on top, but I think it might be nice to collaborate our efforts once we are each satisfied with our implementations.

I have just finished my code, see the GHOP issue page on Google for more information. I will post the documentation shortly.

I corrected a small bug in the size validation part and wrote a nice README file with the complete reference.
Here are the files.

I haven't reviewed this thoroughly, but this isn't actually a Drupal module. You might look at what the previous student posted in #4 as an example of what you should do. I haven't looked at the code there in detail, but it looks like it is written in the form of a Drupal module, and includes a fair amount of phpdoc in the module file itself.

It would also be really helpful if you could include either instructions, or better yet a very simple Drupal module, that can be used to test the code. So you would have your module that is the google chart API module, and then you could write a very simple module that creates some fake data and calls the google chart API module to create the graphs, etc. This second module is not required for the task, but I think it will help people to review your work, and that will probably help you get this finished quicker.

I have written a simple script just to test this, but it's far from being able to be called a module at all. I could post it here later.
And about the previous module, do you mean that I should add more doxygen-style documentation (I know about the ending ?>. I just didn't get to read that discussion until after I posted this)? Or should I also change the mechanism slightly, like replace some code with internal functions just to make it more 'viewable'?
How about the README file?

With regards to what I mean by writing a module, the code that you have written so far does not follow Drupal coding conventions for creating a module. Mostly this is evident in the function names you use, but there may be other places. Modules require .info files that contain information about the module, and most modules implement one or more hooks. I recommend that you start at http://drupal.org/node/292 in the handbook pages and read on from there. I don't think you'll need to do a lot more, but the task description calls for you to create a module, and what you have created so far is not actually a module.

You should also document your code with phpdoc, but that's a separate issue from whether or not the code you've written is actually a module.

I didn't read through the readme thoroughly, but it looks pretty good. But as I suggested before, if you want this to be reviewed quickly, you're going to need to either provide:
1. An example module that actually creates some data and charts, etc.
2. Detailed and to the point instructions on how to review the module.

Most people aren't going to have the time to read through the documentation and come up with code of their own to test that your code works. As I said, I realize that this is not a requirement in the task description, but practically speaking I think it would be good for you to do, unless you can find someone who is already familiar with the Google Chart API and is willing to review and test your code themselves.

Thanks for that. I'll read this document through and fix my work to make it more of a module.
I'll also re-write the testing script to make it an easy to use module. Maybe some GUI frontend module would also help?

On a side, this is my first contribution to, well, anything; but I can walk, I just don't know where to go ;) .

I'm not that familiar with Drupal yet to write filter functionality, but once I'm done I'm really happy to stay with Drupal and maintain my code and docs (and to do much more than just that!).
I'll try to get this done by Monday evening, because I've got lots of homework (it wasn't Christmas holiday, it was our Christmas study break, as my Physics teacher said...) to do, but I'm already on point 6 of the 'Creating modules' tutorial. One thing I just wanted to ask, is if I really need to code all these hooks (for example, hook_perm, I don't see much point in restricting any access to my functionality, or hook_block/hook_node, neither is really what my module is about...)? Also, my module is not meant to configurable, it has some built-in defaults (maybe at some point I will be able to take some defaults from the theme?), and customisation is done on a single-case basis. The idea is so simple, just install it and use it, nothing special.

Okay, as far as now I have managed to get this to work properly. I made the .module and .info files (yet the .info file is the simplest possible, but I don't think going into too much detail is needed here, the module does not require anything really). I have changed the names of the functions, I will add more internal documentation later today. I didn't use hooks, and everything seems to work perfectly with PHP filter. I'm posting this testing code as well as the newer revision of the code, the .info file here just now. I will update the README file tonight or tomorrow.
You can (un)comment any of the lines in the testing code, change the values etc., to see what happens. The chart will be displayed inline wherever you put the code.
The module should be installed as 'googlechart', and will appear as 'Google Chart API' on the modules selection page.
Please tell me if I need to write any additional code (as I said, I will enhance the documentation later), or if I missed something out.

Just responding to a few things.

1. We're glad to have you as a contributor, and have no problem guiding you through the steps and hopefully lead you down the right path.

2. With regards to setting the status of the issue (needs review, needs work, etc.) we typically set an issue to review when there is a patch that is ready to be reviewed (or, in the case of non code tasks, a handbook page or something like that). So, in comment #16 above, you should not have set the status back to needs review, because you had not yet attached the code in module form. Some would argue that in #17 you would also have kept the status at code needs work, since you are in fact still working on it. I'm not trying to be a jerk here--I just want to make sure you understand what the statuses mean and how the community typically uses them, so you can be more effective when you set statuses. And if I don't explain it to you now, somebody else will at some point :) Since you say we should be able to test the code you posted in #17, I'll leave the status as needs review.

3. You're not quite doing phpdoc correctly. Take a look at other modules to see how they do it (core modules tend to be documented the best). For example:

/**
 * Google Chart API
 * wrapper module
 **/
//@see README.txt

should be

/**
 * @file Google Chart API wrapper module.
 *
 * @see README.txt
 */

You should also make sure to follow the Coding standards for Drupal.

I would also suggest that you add phpdoc for each function with a short explanation of the function and a description of the parameters. You don't have to go into huge details for the parameter description, but it's nice to be able to get an idea of what is expected just looking at the code.

As for hooks, etc. you shouldn't need those if your module provides no form of user interface via Drupal, which looks like it's the case.

Just to make sure I understand, for testing one only needs to copy the code in your test.php file into a node using the PHP input format?

I'll try to take a look at this later today and see how it works. Great job so far!

With regards to the statuses, I never knew I was changing anything. I just accepted whatever was in Edit issue settings without touching it (as I wasn't sure how it works). Thanks for pointing that out.
About the phpdoc, I actually wanted to do the exact thing, but I can't quite work on any code right now. I tried to follow the coding standards as much as possible.

For testing you need to install this as a module, enable it, make sure you have PHP input enabled, and then you can just paste the code from test.php into a node, yes.
I have set up an installation of Drupal on my computer, you might want to see http://guarrds.sytes.net/drupal/ , this is what you should see if everything went fine (I sometimes do turn my computer off so it might not always be available.)

I would like to point out that the Chart project is in BETA now.

Restoring title.

tjholowaychuk: I'm glad to know that your module is progressing, but please do not retitle issues (without good reason). It makes it very much harder to keep on track of an issue.

I have improved the phpdoc now, and I'll hopefully find some time today to improve the README file (I found that I forgot to add a few points). I'm attaching the module file just now. I hope there won't be any more changes, and if there will, they will be slight and minor.

@tjholowaychuk - Since you took it upon yourself to work on one of the *scarcely* few tasks that we have available for GHOP students, it would help your karma quite a bit if you were to propose and mentor an alternate task for another student to work on: http://drupal.org/node/add/project-issue/ghop. Thanks.

I know about the ending ?>, I just put it there when I was developing the code, because it was much easier for me to test it outside Drupal during development, and then forgot to remove it. I'll remove it the next time I post the code. Sure, I will replace all the occurrences of colour with color (I always have doubt if I should use the British spelling I'm used to, or the American spelling - the most annoying bit is when I forget and type 'colour' when I'm meant to use 'color', and get errors appearing 'out of nowhere').
And with the concatenating dot, sorry, my bad, I thought you were meant to put the space between the dot and the quote, not between the variable and the dot, and it's actually the opposite way around (it must've been late at night when I was reading the String concatenations chapter of Coding standards). I'll fix that one too, but in some places I was running out of line and decided not to include the space, just so that I did not have to break the line. Should I actually go for the spaces anyway and break the lines?
Thanks for all the support so far. I'll hopefully post the fixed README and module very early tomorrow, so that you can check it ASAP.

I finally tested this out and it looks really cool. I think you're making great progress here.

So, to finish this up (as far as GHOP is concerned), I think you need to finish the following things:

A.) Finish up the documentation and the phpdoc within the module.
B.) Please run your finished module through the coder.module to detect any remaining code style errors. That's much easier than having us point them out as we notice them, a few at a time :)

Unless snufkin comes up with more to add to the list, I think we can give you credit once you're happy with the module and the above two steps are completed.

But I'd like to see a few additions to this module (these would be separate from the GHOP task, and are totally optional):

A.) I think I read somewhere that Google gets mad at you if your site creates more than N charts per day, where N is probably a reasonably large number but one which busy sites might hit. It would be nice if the images were somehow cached locally so that the Google Chart servers weren't hit more frequently than necessary.

B.) I'd love to see more thorough testing/example code. Preferably, you'd provide code to create at least one of each possible type of chart/graph. Doing this would do two things: make it easier for someone to figure out how to use the API quickly, and help to advertise the potential of your module to users.

As I said earlier, these last two things could be worked on after you officially get credit for this task, if you were so inclined.

Thanks for your hard work on this task. Once you're finished, I can see myself using this module on one of my sites.

Ad A, the limit is 50 000 queries per user per day (I'm not fully sure what is meant by 'user', but I guess that'd be a single IP address). Sure, I'm happy to continue improving my module. One of the possible uses would be displaying site statistics (like hits etc.) - the statistics module could detect if Google Chart API module is installed and if so, use it to visualise the data. I would also see my module being accessible from outside the PHP input mode, so that site users could, say, include some small, simple charts in their comments, for example. But as far as now, my priority is to finish off what's required. Sorry, but I won't be able to fix any more code just now, but I'll take my module to school and try to finish it off there (I'm way ahead of rest of class with my coursework ;) ), so that I could just run it through coder.module and send it off to you, and hopefully be done with it.

Here are the updated README and the module files. I ran this through coder.module, corrected all the mistakes (in a few lines it complains about the indenting, because I only put a single space there, to make it clearer, what goes where, when I had to break the lines). The other thing is that it said I shouldn't use camelCaps, but I don't think that's an issue.
Hope that's the final (as in for GHOP) version.
Of course, as I said, I will stay with my module and improve it every so, most importantly add the nice functionality some of you have pointed out.

Very cool. I downloaded and installed the module, and made a quick Drupal 5 style .info file to be able to enable it.

I cobbled together some of your examples from README.txt into a php node, embedding the call to googlechart_getchart in an img tag, and viewed the node. It worked very well. In fact, its very cool.

I see a lot of uses for this.

If you continue to develop this module (into some type of filter that is very accessible to the masses), I predict this will see a lot of use.

And, as a plus, it works fine in HEAD (after creating a D6 style info file for it). I don't know -- and don't see it specified -- what version of Drupal this was to be compatible with.

It does spew a lot of notices in HEAD (attached).

I'll try to review this by the end of tomorrow or earlier. I'll probably be pretty busy at work today and might not have time.

As for CamelCaps, that is technically against the drupal coding standard. I wouldn't let this keep you from getting credit for the task, but I would recommend using variable names like $my_variable instead to be consistent with the rest of Drupal.

I used both camelCaps and the underscore for different situations:
camelCaps for multi-word variable names, but considered as single entities on their own;
underscore to divide the words of a multi-word variable names, where the second word is kind of an adjective (in a sense, of course).
This let me distinguish whether the variable will need any additional processing, or not, for example the $bgOrC variable - it simply contains information whether the script is working on background or chart fill, and all the $url_* variables are to be appended to $url when they are done with processing.
I also know about tjholowaychuk's project. I'm very happy to continue working with him, even if most of my code would be 'wasted'.

Ok, this looks good to me. I'll give you credit for this on the official GHOP task tracker. Nice work. I hope you'll develop this into an actual project on d.o, as this is really good stuff. With regards to the notices, since the original task didn't specify which version of Drupal this should be written for, we'll just assume D5, which is not E_NOTICE compliant anyway :)

Thanks a lot. It was great working for/at/with Drupal. Unfortunately, I'll be away between Saturday the 12th and Tuesday the 29th of January, but then I'll come back to the project and Drupal.