Variable misused used to store state [#696500]

The module uses a variable to store state which results in 2 variable_sets which invalidate the cache table. The tracking to attach the JS or not in the theme can be done using a static variable which removes the need to store a variable in cache fo the lifetime of page. Patch attached.

Comment	File	Size	Author
#1	plus1_misused_variable_set_issue_696500.patch	1.29 KB	voxpelli
	plus1-static.patch	1.41 KB	marcingy

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

voxpelli CreditAttribution: voxpelli commented 4 February 2010 at 13:46

Title:	variable incorrectly used to store state	» Variable misused used to store state
Category:	feature	» bug
Priority:	Normal	» Critical
Status:	Needs review	» Reviewed & tested by the community

File	Size
plus1_misused_variable_set_issue_696500.patch	1.29 KB

This is a pretty critical issue since a variable_set() saves the data to the database as well as clears the variable cache - which results in more pressure on the database.

Plus1 make one database write on every page load as well as one more database write when a widget is shown - which is totally unnecessary and harms performance.

I reviewed the patch - it's pretty basic and it works. I'm attaching a version where I changed the name of the variable to be more descriptive.