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

This website stores cookies on your computer. These cookies are used to improve your website experience and provide more personalized services to you, both on this website and through other media. To find out more about the cookies we use, see our Privacy Policy.