Posted by Scyther on May 18, 2011 at 8:31pm
3 followers
Jump to:
| Project: | Date |
| Version: | 6.x-2.7 |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
<?php
function date_field($op, &$node, $field, &$items, $teaser, $page) {
// Add some information needed to interpret token values.
$additions[$field['field_name']] = $items;
foreach ($items as $delta => $item) {
$timezone = isset($item['timezone']) ? $item['timezone'] : '';
if (is_array($additions[$field['field_name']][$delta])) {
$additions[$field['field_name']][$delta]['timezone'] = date_get_timezone($field['tz_handling'], $timezone);
$additions[$field['field_name']][$delta]['timezone_db'] = date_get_timezone_db($field['tz_handling']);
$additions[$field['field_name']][$delta]['date_type'] = $field['type'];
}
}
switch ($op) {
case 'load':
return $additions;
break;
case 'validate':
require_once('./'. drupal_get_path('module', 'date') .'/date_elements.inc');
return _date_field_validate($op, $node, $field, $items, $teaser, $page);
break;
case 'presave':
case 'insert':
case 'update':
require_once('./'. drupal_get_path('module', 'date') .'/date_elements.inc');
$items = $additions[$field['field_name']];
if ($additions[$field['field_name']]) {
$node->$field['field_name'] = $additions;
}
return _date_field_update($op, $node, $field, $items, $teaser, $page);
break;
case 'sanitize':
foreach ($items as $delta => $item) {
//$dates = date_formatter_process($field, $item, $node, $formatter);
//$node->$field['field_name'][$delta]['dates'] = $dates;
}
break;
}
}
?>This function seems to have some "bad" code in it.
1. sanitize case make a foreach loop and it is doing nothing. Why is this then needed, it only takes up time.
2. only load, presave, insert and update case is using the $additions. So why do a foreach and make that variable for all items when all other $op is not using it? Time and memory that is used unnecessarily.
This code above, that these two points is about, I feel is a bit bad programming code and should be rewriten to have a better preformance on this module.
I also noted that there is a lot of
<?php
require_once('./'. drupal_get_path('module', 'date') .'/...inc');
?>
Comments
#1
That loop in the 'sanitize' case was added in this commit: http://drupalcode.org/project/date.git/commit/b75625fa6966228fa3efe69565...
which references this issue: #223547: Formatter documentation
THEN, the code in the 'sanitize' case was commented out in this commit: http://drupalcode.org/project/date.git/commit/981f96f3516abe02dd16437e39...
But that commit does not reference an issue #...
SO....
I commented out the foreach to eliminate the loop for now:
http://drupalcode.org/project/date.git/commit/08d00cf
#2
#3
Automatically closed -- issue fixed for 2 weeks with no activity.