Moodle

problems with changed number of parameters of validate($date, $files)

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: 1.9
  • Fix Version/s: 1.9
  • Component/s: Forms Library
  • Labels:
    None
  • Affected Branches:
    MOODLE_19_STABLE
  • Fixed Branches:
    MOODLE_19_STABLE

Description

Tim reported some weirdness related to new parameter files in mform->validate(),

I can not reproduce it on my PHP 5.2, the is_validated is calling the validate() with two parameters now, maybe the problem is in method overriding

Tim could you please test following code snippet on your server that shows those errors/warnings:
<?php
require ('config.php');

class a {
function go($c, $d) { echo "a: $c $d <br />"; }
}

class b {
function go($c) { echo "b: $c <br />"; }
}

$a = new a();
$a->go(1, 2);

$b = new b();
$b->go(1, 2);

die;

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Petr Škoda (skodak) added a comment -

Adding Tim...

Show
Petr Škoda (skodak) added a comment - Adding Tim...
Hide
Permalink
Tim Hunt added a comment -

The problem is this: Start with this code:

class a {
function go($c) { echo "a: $c<br />"; }
}

class b extends a {
function go($c) { psrent::go($c); echo "b: $c <br />"; }
}

Then change it to be:

class a {
function go($c, $d) { echo "a: $c $d <br />"; }
}

but don't chage class b. Then you get errors/notices (missing second parameter) when you call b::go and it tries to call a::go.

That is basically what you did when you changed the signature of the validate method in the base class moodleform, but not in any of the other methods that override it.

By the way, following good object oriented design, every subclass of moodleform should start with

$errors = parent::validate($data);

instead of

$errors = array();

That way, if we ever want a global validation hook we have got one. However, only the question type classes (and one or two others if I remember the results of my search) actually do this.

Show
Tim Hunt added a comment - The problem is this: Start with this code: class a { function go($c) { echo "a: $c<br />"; } } class b extends a { function go($c) { psrent::go($c); echo "b: $c <br />"; } } Then change it to be: class a { function go($c, $d) { echo "a: $c $d <br />"; } } but don't chage class b. Then you get errors/notices (missing second parameter) when you call b::go and it tries to call a::go. That is basically what you did when you changed the signature of the validate method in the base class moodleform, but not in any of the other methods that override it. By the way, following good object oriented design, every subclass of moodleform should start with $errors = parent::validate($data); instead of $errors = array(); That way, if we ever want a global validation hook we have got one. However, only the question type classes (and one or two others if I remember the results of my search) actually do this.
Hide
Permalink
Petr Škoda (skodak) added a comment -

The call to parent is a bit problematic because we expected mixed result true/array of errors. I think we did not call parent::validate() in main moodle codebase, so there should be no warnings.

I agree this was not a nice hack, unfortunately I did not find better solution yet. I will go through all of them and fix it. Working on this today...

Show
Petr Škoda (skodak) added a comment - The call to parent is a bit problematic because we expected mixed result true/array of errors. I think we did not call parent::validate() in main moodle codebase, so there should be no warnings. I agree this was not a nice hack, unfortunately I did not find better solution yet. I will go through all of them and fix it. Working on this today...
Hide
Permalink
Tim Hunt added a comment -

I have just checked in a fix for all calls to parent::validation from the question type editing forms. I hope this is a step in the right direction.

Show
Tim Hunt added a comment - I have just checked in a fix for all calls to parent::validation from the question type editing forms. I hope this is a step in the right direction.
Hide
Permalink
Tim Hunt added a comment -

And I have also just checked in a change so that the base class method returns array() instead of true, so you can start your overridden method with

$errors = parent::validation($data, $files);

Show
Tim Hunt added a comment - And I have also just checked in a change so that the base class method returns array() instead of true, so you can start your overridden method with $errors = parent::validation($data, $files);
Hide
Permalink
Petr Škoda (skodak) added a comment -

fix in cvs, thanks for cooperation

Show
Petr Škoda (skodak) added a comment - fix in cvs, thanks for cooperation
Hide
Permalink
Petr Škoda (skodak) added a comment -

Docs in wiki updated too

Show
Petr Škoda (skodak) added a comment - Docs in wiki updated too

People

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.