Posted by sudhaker on September 17, 2008 at 12:15am
Jump to:
| Project: | Profile Privacy |
| Version: | 6.x-1.2 |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (won't fix) |
Issue Summary
I guess INSERT is redundant when $privacy is 0 ; so why bother...
Existing code:
function profile_privacy_set_field_privacy($field_name, $privacy = 0) {
profile_privacy_get_fields(NULL, TRUE);
$field = profile_privacy_get_field($field_name);
db_query('DELETE FROM {profile_privacy_fields} WHERE fid = %d', $field->fid);
db_query('INSERT INTO {profile_privacy_fields} (fid, privacy) VALUES (%d, %d)', $field->fid, $privacy);
}Suggestion: change in last statement
function profile_privacy_set_field_privacy($field_name, $privacy = 0) {
profile_privacy_get_fields(NULL, TRUE);
$field = profile_privacy_get_field($field_name);
db_query('DELETE FROM {profile_privacy_fields} WHERE fid = %d', $field->fid);
if ($privacy > 0) db_query('INSERT INTO {profile_privacy_fields} (fid, privacy) VALUES (%d, %d)', $field->fid, $privacy);
}Cheers,
Sudhaker
Comments
#1
@maintainer, plz review and if useful implement that would be appreciated.
#2
sudhaker,
Thanks for the contribution. Although this would save an occasional SQL query and would be an optimization, I am not sure it is a critical to get into the module. Also, the 6.x-2.x branch changes this field from an integer to a varchar field to allow for multiple privacy settings, so I don't want to introduce something into the 1.x branch that will be deprecated going forward. I do like your thinking here, though.
Also as a minor nit-pick, review the coding standards for control structures.
Respectfully marking this as "won't fix".
Thanks,
Chris