Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-14055

code uses hard-coded integer values instead of defined, human-readable strings

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Trivial
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.0
    • Component/s: Authentication
    • Labels:
      None
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE

      Description

      What does "2" mean? I don't know. Ah – look at the HTML:

      <option value="0" selected="selected">Keep internal</option>
      <option value="1">Suspend internal</option>
      <option value="2">Full delete internal</option>

      The code should use, e.g., AUTH_REMOVEUSER_KEEP, AUTH_REMOVEUSER_SUSPEND, and AUTH_REMOVEUSER_FULLDELETE instead of 0, 1, and 2.

      I'll submit a patch for 2.0 shortly, because I suppose it's not worth changing released versions. This will NOT change functionality, it will only improve code readability (which should cut down on future bug introduction, developer frustration, etc.)

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            moquist Matt Oquist added a comment -

            Here's a patch against HEAD. Warning: this is untested, though a visual inspection of the diff shows the hard-coded values replaced cleanly with the define()d tokens, so I don't know how anything could go wrong.

            I'm just about to start testing user removal with external db auth on my 1.8.3+ test system, and I made this patch from the changes I've already made there. I'll definitely comment here if I run into any problems.

            Show
            moquist Matt Oquist added a comment - Here's a patch against HEAD. Warning: this is untested, though a visual inspection of the diff shows the hard-coded values replaced cleanly with the define()d tokens, so I don't know how anything could go wrong. I'm just about to start testing user removal with external db auth on my 1.8.3+ test system, and I made this patch from the changes I've already made there. I'll definitely comment here if I run into any problems.
            Hide
            dougiamas Martin Dougiamas added a comment -

            Thanks Matt! Dongsheng can you check in to dev ?

            Show
            dougiamas Martin Dougiamas added a comment - Thanks Matt! Dongsheng can you check in to dev ?
            Hide
            andyjdavis Andrew Davis added a comment -

            verified. closing. While there i noticed this same problem elsewhere. MDL-21066

            Show
            andyjdavis Andrew Davis added a comment - verified. closing. While there i noticed this same problem elsewhere. MDL-21066

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  24/Nov/10