Moodle
  1. Moodle
  2. MDL-12133

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

    Details

    • Type: Bug Bug
    • Status: 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
    • Rank:
      29934

      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

        Hide
        Petr Škoda added a comment -

        Adding Tim...

        Show
        Petr Škoda added a comment - Adding Tim...
        Hide
        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
        Petr Škoda 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 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
        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
        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
        Petr Škoda added a comment -

        fix in cvs, thanks for cooperation

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

        Docs in wiki updated too

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

          People

          • Assignee:
            Petr Škoda
            Reporter:
            Petr Škoda
            Tester:
            Nobody
            Participants:
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: