remove nonsense default value #972

Merged
merged 1 commit into from Nov 24, 2012

Conversation

Projects
None yet
3 participants
Member

dereuromark commented Nov 22, 2012

and

  • some doc block corrections
  • and one !! to (bool) change

@ADmad ADmad commented on the diff Nov 22, 2012

lib/Cake/Utility/Folder.php
@@ -634,7 +634,7 @@ public function delete($path = null) {
* @return boolean Success
* @link http://book.cakephp.org/2.0/en/core-utility-libraries/file-folder.html#Folder::copy
*/
- public function copy($options = array()) {
+ public function copy($options) {
@ADmad

ADmad Nov 22, 2012

Member

Why did you remove the default value? Just because the param can take a string value too doesn't mean it cannot have array() as default value.

@dereuromark

dereuromark Nov 22, 2012

Member

move() doesnt have a default value either. and both shoudnt have. they make no sense without since you always need to pass sth to make it actually do sth. You need a "target" - either way.
Also - removing the default doesnt make it NOT take a string as a value.

@ADmad

ADmad Nov 22, 2012

Member

I didn't read the function code earlier just saw the docblock. An empty array would mean no value being set for destination directory so this change does make sense.

@dereuromark

dereuromark Nov 22, 2012

Member

thats the point. you NEED to pass some value here. you cannot just run around putting

$Folder->copy();
$Folder->copy();
$Folder->copy();
$Folder->copy();

in your code (which would be possible with the default value in place - but would just be a big junk of garbige code).

you need to at least do

$Folder->copy($var);

even if that one is then NULL, string or array then.

please see the move() method for clarification. this one doesnt have the (ridiculous) default value.

@ADmad

ADmad Nov 22, 2012

Member

if you still not understand the purpose, ...

In my previous comment I already said "..this change does make sense."

@dereuromark

dereuromark Nov 23, 2012

Member

haha, sry, for that - even after reading your comment twice i still read "doesnt make sense" :-)

Member

ADmad commented Nov 22, 2012

👎 I would prefer not going overboard with type hinting and using it only for objects.

Member

dereuromark commented Nov 22, 2012

all right, no type hints for arrays then
the rest should be fine now!

Owner

markstory commented Nov 22, 2012

The changes as they stand look good to me.

@markstory markstory added a commit that referenced this pull request Nov 24, 2012

@markstory markstory Merge pull request #972 from dereuromark/2.3-type-hinting
Remove nonsense default value.
e7c3034

@markstory markstory merged commit e7c3034 into cakephp:2.3 Nov 24, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment