[Mahara-contributors] [Bug 1620416] Re: Use of assign_by_ref() is not clear as to what is required

2016-10-20 Thread Robert Lyon
** Changed in: mahara
   Status: Fix Committed => Fix Released

-- 
You received this bug notification because you are a member of Mahara
Contributors, which is subscribed to Mahara.
Matching subscriptions: Subscription for all Mahara Contributors -- please ask 
on #mahara-dev or mahara.org forum before editing or unsubscribing it!
https://bugs.launchpad.net/bugs/1620416

Title:
  Use of assign_by_ref() is not clear as to what is required

Status in Mahara:
  Fix Released

Bug description:
  In Mahara we have a bunch of $smarty->assign_by_ref('item',
  $variable);

  It was originally added to smarty/dwoo due to the following

  "The assign_by_ref() original intention in Smarty 2 was to work around
  the object-by-copy behavior of PHP4."

  "The _by_ref methods have been introduced in Smarty2 mainly to be able
  to pass objects to the templates in PHP4. In PHP5 these are passed
  alway as a reference."

  But it doesn't look like we use them in a true reference sort of way

  What I mean is, this example shows referenced vs not referenced
  'title' variable:

  $smarty = smarty();
  $title = 'cats';
  $smarty->assign('title', $title);
  $smarty->assign_by_ref('titleref', $title);
  $title = 'dogs';
  $smarty->display('template.tpl'); 

  In the template it will display 'cat' as title and 'dogs' as titleref
  rather than 'cat'.

  We don't support PHP4 and so should clean up the code and make the
  assign_by_ref() calls simply assign() where appropriate to make the
  code clear as to what we are wanting.

To manage notifications about this bug go to:
https://bugs.launchpad.net/mahara/+bug/1620416/+subscriptions

___
Mailing list: https://launchpad.net/~mahara-contributors
Post to : mahara-contributors@lists.launchpad.net
Unsubscribe : https://launchpad.net/~mahara-contributors
More help   : https://help.launchpad.net/ListHelp


[Mahara-contributors] [Bug 1620416] Re: Use of assign_by_ref() is not clear as to what is required

2016-09-22 Thread Robert Lyon
** Changed in: mahara
   Status: In Progress => Fix Committed

-- 
You received this bug notification because you are a member of Mahara
Contributors, which is subscribed to Mahara.
Matching subscriptions: Subscription for all Mahara Contributors -- please ask 
on #mahara-dev or mahara.org forum before editing or unsubscribing it!
https://bugs.launchpad.net/bugs/1620416

Title:
  Use of assign_by_ref() is not clear as to what is required

Status in Mahara:
  Fix Committed

Bug description:
  In Mahara we have a bunch of $smarty->assign_by_ref('item',
  $variable);

  It was originally added to smarty/dwoo due to the following

  "The assign_by_ref() original intention in Smarty 2 was to work around
  the object-by-copy behavior of PHP4."

  "The _by_ref methods have been introduced in Smarty2 mainly to be able
  to pass objects to the templates in PHP4. In PHP5 these are passed
  alway as a reference."

  But it doesn't look like we use them in a true reference sort of way

  What I mean is, this example shows referenced vs not referenced
  'title' variable:

  $smarty = smarty();
  $title = 'cats';
  $smarty->assign('title', $title);
  $smarty->assign_by_ref('titleref', $title);
  $title = 'dogs';
  $smarty->display('template.tpl'); 

  In the template it will display 'cat' as title and 'dogs' as titleref
  rather than 'cat'.

  We don't support PHP4 and so should clean up the code and make the
  assign_by_ref() calls simply assign() where appropriate to make the
  code clear as to what we are wanting.

To manage notifications about this bug go to:
https://bugs.launchpad.net/mahara/+bug/1620416/+subscriptions

___
Mailing list: https://launchpad.net/~mahara-contributors
Post to : mahara-contributors@lists.launchpad.net
Unsubscribe : https://launchpad.net/~mahara-contributors
More help   : https://help.launchpad.net/ListHelp


[Mahara-contributors] [Bug 1620416] Re: Use of assign_by_ref() is not clear as to what is required

2016-09-20 Thread Cecilia Vela Gurovic
To test this fix, the results before and after applying the changes
should be the same. We need to test that the functionality that uses the
data passed by reference (assign_by_ref) is not changed when we use the
assign() instead.

-- 
You received this bug notification because you are a member of Mahara
Contributors, which is subscribed to Mahara.
Matching subscriptions: Subscription for all Mahara Contributors -- please ask 
on #mahara-dev or mahara.org forum before editing or unsubscribing it!
https://bugs.launchpad.net/bugs/1620416

Title:
  Use of assign_by_ref() is not clear as to what is required

Status in Mahara:
  In Progress

Bug description:
  In Mahara we have a bunch of $smarty->assign_by_ref('item',
  $variable);

  It was originally added to smarty/dwoo due to the following

  "The assign_by_ref() original intention in Smarty 2 was to work around
  the object-by-copy behavior of PHP4."

  "The _by_ref methods have been introduced in Smarty2 mainly to be able
  to pass objects to the templates in PHP4. In PHP5 these are passed
  alway as a reference."

  But it doesn't look like we use them in a true reference sort of way

  What I mean is, this example shows referenced vs not referenced
  'title' variable:

  $smarty = smarty();
  $title = 'cats';
  $smarty->assign('title', $title);
  $smarty->assign_by_ref('titleref', $title);
  $title = 'dogs';
  $smarty->display('template.tpl'); 

  In the template it will display 'cat' as title and 'dogs' as titleref
  rather than 'cat'.

  We don't support PHP4 and so should clean up the code and make the
  assign_by_ref() calls simply assign() where appropriate to make the
  code clear as to what we are wanting.

To manage notifications about this bug go to:
https://bugs.launchpad.net/mahara/+bug/1620416/+subscriptions

___
Mailing list: https://launchpad.net/~mahara-contributors
Post to : mahara-contributors@lists.launchpad.net
Unsubscribe : https://launchpad.net/~mahara-contributors
More help   : https://help.launchpad.net/ListHelp


[Mahara-contributors] [Bug 1620416] Re: Use of assign_by_ref() is not clear as to what is required

2016-09-06 Thread Cecilia Vela Gurovic
** Changed in: mahara
   Status: New => In Progress

-- 
You received this bug notification because you are a member of Mahara
Contributors, which is subscribed to Mahara.
Matching subscriptions: Subscription for all Mahara Contributors -- please ask 
on #mahara-dev or mahara.org forum before editing or unsubscribing it!
https://bugs.launchpad.net/bugs/1620416

Title:
  Use of assign_by_ref() is not clear as to what is required

Status in Mahara:
  In Progress

Bug description:
  In Mahara we have a bunch of $smarty->assign_by_ref('item',
  $variable);

  It was originally added to smarty/dwoo due to the following

  "The assign_by_ref() original intention in Smarty 2 was to work around
  the object-by-copy behavior of PHP4."

  "The _by_ref methods have been introduced in Smarty2 mainly to be able
  to pass objects to the templates in PHP4. In PHP5 these are passed
  alway as a reference."

  But it doesn't look like we use them in a true reference sort of way

  What I mean is, this example shows referenced vs not referenced
  'title' variable:

  $smarty = smarty();
  $title = 'cats';
  $smarty->assign('title', $title);
  $smarty->assign_by_ref('titleref', $title);
  $title = 'dogs';
  $smarty->display('template.tpl'); 

  In the template it will display 'cat' as title and 'dogs' as titleref
  rather than 'cat'.

  We don't support PHP4 and so should clean up the code and make the
  assign_by_ref() calls simply assign() where appropriate to make the
  code clear as to what we are wanting.

To manage notifications about this bug go to:
https://bugs.launchpad.net/mahara/+bug/1620416/+subscriptions

___
Mailing list: https://launchpad.net/~mahara-contributors
Post to : mahara-contributors@lists.launchpad.net
Unsubscribe : https://launchpad.net/~mahara-contributors
More help   : https://help.launchpad.net/ListHelp


[Mahara-contributors] [Bug 1620416] Re: Use of assign_by_ref() is not clear as to what is required

2016-09-06 Thread Cecilia Vela Gurovic
** Changed in: mahara
 Assignee: (unassigned) => Cecilia Vela Gurovic (ceciliavg)

-- 
You received this bug notification because you are a member of Mahara
Contributors, which is subscribed to Mahara.
Matching subscriptions: Subscription for all Mahara Contributors -- please ask 
on #mahara-dev or mahara.org forum before editing or unsubscribing it!
https://bugs.launchpad.net/bugs/1620416

Title:
  Use of assign_by_ref() is not clear as to what is required

Status in Mahara:
  New

Bug description:
  In Mahara we have a bunch of $smarty->assign_by_ref('item',
  $variable);

  It was originally added to smarty/dwoo due to the following

  "The assign_by_ref() original intention in Smarty 2 was to work around
  the object-by-copy behavior of PHP4."

  "The _by_ref methods have been introduced in Smarty2 mainly to be able
  to pass objects to the templates in PHP4. In PHP5 these are passed
  alway as a reference."

  But it doesn't look like we use them in a true reference sort of way

  What I mean is, this example shows referenced vs not referenced
  'title' variable:

  $smarty = smarty();
  $title = 'cats';
  $smarty->assign('title', $title);
  $smarty->assign_by_ref('titleref', $title);
  $title = 'dogs';
  $smarty->display('template.tpl'); 

  In the template it will display 'cat' as title and 'dogs' as titleref
  rather than 'cat'.

  We don't support PHP4 and so should clean up the code and make the
  assign_by_ref() calls simply assign() where appropriate to make the
  code clear as to what we are wanting.

To manage notifications about this bug go to:
https://bugs.launchpad.net/mahara/+bug/1620416/+subscriptions

___
Mailing list: https://launchpad.net/~mahara-contributors
Post to : mahara-contributors@lists.launchpad.net
Unsubscribe : https://launchpad.net/~mahara-contributors
More help   : https://help.launchpad.net/ListHelp


[Mahara-contributors] [Bug 1620416] Re: Use of assign_by_ref() is not clear as to what is required

2016-09-06 Thread Aaron Wells
Here's the function declarations (in
htdocs/lib/dwoo/mahara/Dwoo_Mahara.php):

/**
 * implements smarty api to assign data
 */
public function assign($key, $value) {
$this->_data[$key] = $value;
}

/**
 * implements smarty api to assign data
 */
public function assign_by_ref($key, &$value) {
$this->_data[$key] =& $value;
}

I guess it makes sense, the difference is that assign_by_ref() passes
the parameter by reference (note the "&") and assign() passes it the PHP
default way, by value.

There are generally two reasons why you might pass a parameter by
reference in PHP. The primary reason is if you want the function to be
able to modify its parameters, usually as a way to return multiple
values. But in the case of Dwoo, that's unlikely to be useful because we
compile these templates last and we only use them for display purposes.

The other reason is for optimization. If a parameter is very large, then
you might want to pass it by reference in order to avoid having to make
a copy of it. That's probably where PHP 4 comes into this, as well. In
PHP 4, passing an object as a parameter would make a copy of the object.
In PHP 5 the handling of objects changed, so that variables hold a
pointer to the object instead of the actual object-in-memory itself.
Consequently, in PHP5 objects passed to functions are already handled
rather similarly to if they were passed by reference. (Though not
exactly the same. See here for the gory details:
http://php.net/manual/en/language.oop5.references.php ). So we don't
need $smarty->assign_by_ref() to handle objects anymore.

Additionally, using references for optimization is very 2008. PHP
engines from 5+ are optimized so that passing by value is actually
*faster* unless you're passing *very* large strings or arrays, and even
then it's only a modest speed improvement noticeable if you're doing it
thousands of times per page. We have at most a couple dozen
$smarty->assign() calls on a page. (See
http://stackoverflow.com/questions/178328/in-php-5-0-is-passing-by-
reference-faster ) So now the universal advice is to *only* use pass-by-
reference if you need to modify parameters, not as an attempt at
optimization.

So where does that leave us? Well, since we don't want Dwoo templates
modifying the values passed to them, we really should be using
$smarty->assign() everywhere instead of $smarty->assign_by_ref(). It
probably would be an okay idea to clean up all the old
$smarty->assign_by_ref() calls and change them to $smarty->assign(). The
one possible exception is if we're passing an enormous array to smarty,
like something that could cause an out-of-memory error if it were
duplicated. Although even then, it would probably be best to use other
means to avoid having that whole array in memory in the first place,
like using a file iterator or a database results pointer.

-- 
You received this bug notification because you are a member of Mahara
Contributors, which is subscribed to Mahara.
Matching subscriptions: Subscription for all Mahara Contributors -- please ask 
on #mahara-dev or mahara.org forum before editing or unsubscribing it!
https://bugs.launchpad.net/bugs/1620416

Title:
  Use of assign_by_ref() is not clear as to what is required

Status in Mahara:
  New

Bug description:
  In Mahara we have a bunch of $smarty->assign_by_ref('item',
  $variable);

  It was originally added to smarty/dwoo due to the following

  "The assign_by_ref() original intention in Smarty 2 was to work around
  the object-by-copy behavior of PHP4."

  "The _by_ref methods have been introduced in Smarty2 mainly to be able
  to pass objects to the templates in PHP4. In PHP5 these are passed
  alway as a reference."

  But it doesn't look like we use them in a true reference sort of way

  What I mean is, this example shows referenced vs not referenced
  'title' variable:

  $smarty = smarty();
  $title = 'cats';
  $smarty->assign('title', $title);
  $smarty->assign_by_ref('titleref', $title);
  $title = 'dogs';
  $smarty->display('template.tpl'); 

  In the template it will display 'cat' as title and 'dogs' as titleref
  rather than 'cat'.

  We don't support PHP4 and so should clean up the code and make the
  assign_by_ref() calls simply assign() where appropriate to make the
  code clear as to what we are wanting.

To manage notifications about this bug go to:
https://bugs.launchpad.net/mahara/+bug/1620416/+subscriptions

___
Mailing list: https://launchpad.net/~mahara-contributors
Post to : mahara-contributors@lists.launchpad.net
Unsubscribe : https://launchpad.net/~mahara-contributors
More help   : https://help.launchpad.net/ListHelp


[Mahara-contributors] [Bug 1620416] Re: Use of assign_by_ref() is not clear as to what is required

2016-09-05 Thread Robert Lyon
When I change the lib/web.php line

 $smarty->assign_by_ref('JAVASCRIPT', $javascript_array);

to

 $smarty->assign('JAVASCRIPT', $javascript_array);

The resulting dataroot/dwoo/compile/.../htdocs/mahara-
testing/mahara/htdocs/theme/raw/templates/header/head.tpl.d17.php looks
to be the same for either option

-- 
You received this bug notification because you are a member of Mahara
Contributors, which is subscribed to Mahara.
Matching subscriptions: Subscription for all Mahara Contributors -- please ask 
on #mahara-dev or mahara.org forum before editing or unsubscribing it!
https://bugs.launchpad.net/bugs/1620416

Title:
  Use of assign_by_ref() is not clear as to what is required

Status in Mahara:
  New

Bug description:
  In Mahara we have a bunch of $smarty->assign_by_ref('item',
  $variable);

  It was originally added to smarty/dwoo due to the following

  "The assign_by_ref() original intention in Smarty 2 was to work around
  the object-by-copy behavior of PHP4."

  "The _by_ref methods have been introduced in Smarty2 mainly to be able
  to pass objects to the templates in PHP4. In PHP5 these are passed
  alway as a reference."

  But it doesn't look like we use them in a true reference sort of way

  What I mean is, this example shows referenced vs not referenced
  'title' variable:

  $smarty = smarty();
  $title = 'cats';
  $smarty->assign('title', $title);
  $smarty->assign_by_ref('titleref', $title);
  $title = 'dogs';
  $smarty->display('template.tpl'); 

  In the template it will display 'cat' as title and 'dogs' as titleref
  rather than 'cat'.

  We don't support PHP4 and so should clean up the code and make the
  assign_by_ref() calls simply assign() where appropriate to make the
  code clear as to what we are wanting.

To manage notifications about this bug go to:
https://bugs.launchpad.net/mahara/+bug/1620416/+subscriptions

___
Mailing list: https://launchpad.net/~mahara-contributors
Post to : mahara-contributors@lists.launchpad.net
Unsubscribe : https://launchpad.net/~mahara-contributors
More help   : https://help.launchpad.net/ListHelp