Moodle
  1. Moodle
  2. MDL-14771

Allow a "number" field to be displayed with a specified number of decimals (Backend code)

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.9, 1.9.1
    • Fix Version/s: 1.9.2
    • Labels:
      None
    • Database:
      Any
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Rank:
      33798

      Description

      Currently, a field of type "number" is useless, since there is no advantage compared to a simple "text" field. We all know what Excel or OpenOffice's Calc offers for a number type field. To make the number field useful, there should at least be the possibility to display a number with a given number of decimal digits.

      1. MDL-14771_20080512.patch
        1 kB
        Robert Allerstorfer
      2. MDL-14771_20080530.patch
        1 kB
        Robert Allerstorfer

        Issue Links

          Activity

          Hide
          Robert Allerstorfer added a comment - - edited

          The attached patch allows specifying a number (integer >= 0) of decimal digits a number should be displayed with. The number of decimals can be specified within the "preset.xml" file (which is part of a "preset.zip" preset), as "param1". For example, to define a number field for a Moisture in % with 2 decimals, we can use:

          <field>
          <type>number</type>
          <name>Moisture</name>
          <description>Moisture (in %)</description>
          <param1>2</param1>
          </field>

          For example, a stored number of 0.999 will be displayed as 1.00 and 0.002 will be displayed as 0.00.

          Show
          Robert Allerstorfer added a comment - - edited The attached patch allows specifying a number (integer >= 0) of decimal digits a number should be displayed with. The number of decimals can be specified within the "preset.xml" file (which is part of a "preset.zip" preset), as "param1". For example, to define a number field for a Moisture in % with 2 decimals, we can use: <field> <type>number</type> <name>Moisture</name> <description>Moisture (in %)</description> <param1>2</param1> </field> For example, a stored number of 0.999 will be displayed as 1.00 and 0.002 will be displayed as 0.00.
          Hide
          Robert Allerstorfer added a comment -

          The updated version of the patch (MDL-14771_20080512.patch) treats fields containing the number 0 properly. Works perfectly together with the patch I submitted today at MDL-14788.

          Show
          Robert Allerstorfer added a comment - The updated version of the patch ( MDL-14771 _20080512.patch) treats fields containing the number 0 properly. Works perfectly together with the patch I submitted today at MDL-14788 .
          Hide
          Robert Allerstorfer added a comment -

          Now that the MDL-14788 bugfix has been committed and verified, this patch should get committed asap, too. The currently proposed patch does not change any current functionality, so its completely harmless to commit it. The big advantage is that users can start to use the "hidden" feature by importing a well-cared "preset.zip" containing a "preset.xml" with manually added <param1></param1> tags in number fields.

          After this patch has been committed, a GUI should be added to the "Number field" editing page accessable via the "Fields" tab, however, there is no need to add this asap.

          Show
          Robert Allerstorfer added a comment - Now that the MDL-14788 bugfix has been committed and verified, this patch should get committed asap, too. The currently proposed patch does not change any current functionality, so its completely harmless to commit it. The big advantage is that users can start to use the "hidden" feature by importing a well-cared "preset.zip" containing a "preset.xml" with manually added <param1></param1> tags in number fields. After this patch has been committed, a GUI should be added to the "Number field" editing page accessable via the "Fields" tab, however, there is no need to add this asap.
          Hide
          Robert Allerstorfer added a comment -

          The backend code has now been committed, so people can start using this feature via presets. A new issue should be created regarding the frontend code (addition to the GUI).

          Show
          Robert Allerstorfer added a comment - The backend code has now been committed, so people can start using this feature via presets. A new issue should be created regarding the frontend code (addition to the GUI).
          Hide
          Petr Škoda added a comment - - edited

          hi,

          while reviewing the commit I noticed you are using number_float()
          we already have format_float() and unformat_float() - I guess those should be used instead

          also
          $decimals = intval($this->field->param1);
          if (isset($decimals) && is_int($decimals) && $decimals >= 0) {

          might not be what you wanted because it is always set and always integer, right?

          Show
          Petr Škoda added a comment - - edited hi, while reviewing the commit I noticed you are using number_float() we already have format_float() and unformat_float() - I guess those should be used instead also $decimals = intval($this->field->param1); if (isset($decimals) && is_int($decimals) && $decimals >= 0) { might not be what you wanted because it is always set and always integer, right?
          Hide
          Robert Allerstorfer added a comment -

          Hi Petr,

          (1) No, I don't use number_float(). The function I am using to format the number is number_format() - a standard PHP function (http://php.net/manual/en/function.number-format.php).

          (2) Yes, you are right - my integer checking was not useful - the attached patch (MDL-14771_integer-check.patch) corrects this. I have tested it successfully with many different values set in param1, and of course with nothing set in param1 at all.

          Please review again, thanks.

          Show
          Robert Allerstorfer added a comment - Hi Petr, (1) No, I don't use number_float(). The function I am using to format the number is number_format() - a standard PHP function ( http://php.net/manual/en/function.number-format.php ). (2) Yes, you are right - my integer checking was not useful - the attached patch ( MDL-14771 _integer-check.patch) corrects this. I have tested it successfully with many different values set in param1, and of course with nothing set in param1 at all. Please review again, thanks.
          Hide
          Petr Škoda added a comment -

          oops, sorry for the wrong function name - all new code in 1.9.x should be using xxx_float() for float processing, in 2.0 it will be mandatory because we need to support languages that use "floating comma" numbers

          Show
          Petr Škoda added a comment - oops, sorry for the wrong function name - all new code in 1.9.x should be using xxx_float() for float processing, in 2.0 it will be mandatory because we need to support languages that use "floating comma" numbers
          Hide
          Petr Škoda added a comment -

          1/ ctype_digit not allowed, extension might not be present
          2/ when dealing with decimal separators, it must be done both before input, after input and before display

          Show
          Petr Škoda added a comment - 1/ ctype_digit not allowed, extension might not be present 2/ when dealing with decimal separators, it must be done both before input, after input and before display
          Hide
          Robert Allerstorfer added a comment -

          I do not do float processing, so I don't understand why you mention that here.

          Moodle 1.9 requires PHP 4.3.0 or later, and beginning with that version ctype support is built in into PHP's core (see http://php.net/manual/en/ctype.installation.php). Thus, the function ctype_digit() which I use to check the value of param1 should always be available.

          What relationship has your last comment (regarding decimal separators) to the proposed patch?

          Show
          Robert Allerstorfer added a comment - I do not do float processing, so I don't understand why you mention that here. Moodle 1.9 requires PHP 4.3.0 or later, and beginning with that version ctype support is built in into PHP's core (see http://php.net/manual/en/ctype.installation.php ). Thus, the function ctype_digit() which I use to check the value of param1 should always be available. What relationship has your last comment (regarding decimal separators) to the proposed patch?
          Hide
          Petr Škoda added a comment -

          1/ ok those are two different problems, but anyway do not use number_format() directly here
          2/ ctype function caused problems on some servers (MDL-3381), which means it can not be used in 1.9.x - sorry

          Show
          Petr Škoda added a comment - 1/ ok those are two different problems, but anyway do not use number_format() directly here 2/ ctype function caused problems on some servers ( MDL-3381 ), which means it can not be used in 1.9.x - sorry
          Hide
          Robert Allerstorfer added a comment -

          OK, the new patch ("MDL-14771_integer-check-preg_match.patch") does not use a ctype function. Instead of ctype_digit() I am now using preg_match(). I prefer using the allmighty PCREs anyway

          However, PCRE support can be disabled in PHP versions < 5.3. PCREs are used in many Moodle files, but the environment check does not mention it as being required (nor as recommended). Should we file a bug on this?

          It seems that now the only thing you don't like in my proposed patch is the number_format() PHP core function. What is bad with it?

          Show
          Robert Allerstorfer added a comment - OK, the new patch (" MDL-14771 _integer-check-preg_match.patch") does not use a ctype function. Instead of ctype_digit() I am now using preg_match(). I prefer using the allmighty PCREs anyway However, PCRE support can be disabled in PHP versions < 5.3. PCREs are used in many Moodle files, but the environment check does not mention it as being required (nor as recommended). Should we file a bug on this? It seems that now the only thing you don't like in my proposed patch is the number_format() PHP core function. What is bad with it?
          Hide
          Petr Škoda added a comment -

          1/ PCRE - yes please file it

          2/ I thought you would figure that out yourself.

          Show
          Petr Škoda added a comment - 1/ PCRE - yes please file it 2/ I thought you would figure that out yourself.
          Hide
          Robert Allerstorfer added a comment -

          ad 1: bug filed as MDL-15057.
          ad 2: number_format() is used in 17 other Moodle core files. The function works fine - if you have a reason to forbid it in Moodle code, it should be removed from the existing code as well. No idea what should be wrong with it.

          Show
          Robert Allerstorfer added a comment - ad 1: bug filed as MDL-15057 . ad 2: number_format() is used in 17 other Moodle core files. The function works fine - if you have a reason to forbid it in Moodle code, it should be removed from the existing code as well. No idea what should be wrong with it.
          Hide
          Petr Škoda added a comment - - edited

          please read: "all new code in 1.9.x should be using xxx_float() ...." above (xxx_float() means format_float() or unformat_float())

          Show
          Petr Škoda added a comment - - edited please read: "all new code in 1.9.x should be using xxx_float() ...." above (xxx_float() means format_float() or unformat_float())
          Hide
          Petr Škoda added a comment -

          those 17 hits are either from code that was not converted yet (quiz and lesson) or imported libraries (code from adodb and typo3 not used, the graphlib should be revisited in 2.0)
          I spent some time getting rid of these and it annoys me a bit when you keep trying to push it into cvs.

          I guess it is partly my fault because I did not add it to coding style, fixed now - see http://docs.moodle.org/en/Development:Coding#General_rules

          Thanks for working on this!

          Show
          Petr Škoda added a comment - those 17 hits are either from code that was not converted yet (quiz and lesson) or imported libraries (code from adodb and typo3 not used, the graphlib should be revisited in 2.0) I spent some time getting rid of these and it annoys me a bit when you keep trying to push it into cvs. I guess it is partly my fault because I did not add it to coding style, fixed now - see http://docs.moodle.org/en/Development:Coding#General_rules Thanks for working on this!
          Hide
          Robert Allerstorfer added a comment -

          OK, now I understand what you have meant. Is the new pacth (MDL-14771_20080530.patch) now OK for you?

          Show
          Robert Allerstorfer added a comment - OK, now I understand what you have meant. Is the new pacth ( MDL-14771 _20080530.patch) now OK for you?
          Hide
          Petr Škoda added a comment -

          ok for me

          Show
          Petr Škoda added a comment - ok for me
          Hide
          Robert Allerstorfer added a comment -

          Thanks for reviewing!

          Show
          Robert Allerstorfer added a comment - Thanks for reviewing!

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: