Forums

Code Review

Permalink 1 user found helpful
I wrote a tutorial yesterday showing users how to edit the chart_handler.php file to show google analytics data in the dashboard graph. I was hoping that someone with a firm grasp on php could look over my code and give me any notes. I just want to provide c5 users with a well written solution, and make sure I didn't commit any serious coding injustices.

I will probably follow this tutorial with another that moves the config into a dashboard form, and packages it all up.

Which reminds me. In this situation we have to store the users password for login to google. Is there a best practice for storing passwords in the database?

Thanks for any help and comments!

http://www.concrete5tutorials.com/block-tutorials/configure-the-con...

-Guy
guythomas
 
tbcrowe replied on at Permalink Reply
tbcrowe
Guy,

This is a great idea for a tutorial. I ran through it with a site of mine and it worked great. I only have a couple very minor comments about your code:
1) There's no need to rename the gapi.class.php file. It will work find as is.
2) You use unquoted string literals in a number of places for array indices (e.g. $days_stats[visits]). While PHP will work that way, it does cause notice statements to appear in your php log file if you have notices enabled. You should always quote your string literals ($days_stats['visits']).

One other suggestion: this can take a little while for this to run. It would be nice if the graph data were cached so that it only had to be retrieved after some period of time (say 5 minutes).

- Todd
guythomas replied on at Permalink Reply
guythomas
Thanks for taking a look at it Todd. I appreciate your feedback.

I changed it up as you suggested, to include caching the output for speed. Only about 5 additional lines of code.. gotta love C5!

-Guy
katz515 replied on at Permalink Reply
katz515
Nice work!

I just translated your post onto Japanese in our users forum
http://concrete5-japan.org/community/forums/development/post-1624/...

Cheers.
guythomas replied on at Permalink Reply
guythomas
I appreciate that katz.. You're the man!
-guy