Remove the mis-used define()'s

The reason we use named constants for variables and permissions is sound programming practices: you want to use named constants rather than literals repeated in many places where ever you can.

This has been advocated for decades regardless of the language used or application, as it makes maintenance easier, and reduces errors. The alternative is peppering the code with literals, and it becomes very hard to debug if you mispell by one letter.

With the constants, at least you get a notice if the variable is not defined.

If a string or magic number is used in one one place, then it is fine to just used it as a string. If it is used in more than one place, then it is better to abstract it using named constants.

See more here, here and here.

kbahey: and that approach was dubbed inadequate for Drupal. See http://drupal.org/node/260226.

I've been complaining about UP's constant usage for as long as I can remember, and chx and I continue to promote sensible constant usage in the Drupal Tough Love posts. The most key point is: constants should clarify, not replicate. A healthy portion of your constants (all the ones dmitrig01 removed, actually) merely replicate. He left the ones that clarified.

I have to agree with Morbus. I'll give a +1 for the patch.
Though i can see why kbahey promotes it, it is like a verbose mode. You dont need it, unless you are developing. And if you are developing, and testing well, you're going to see where it goes wrong. But anyway, you dont need it in live code.

This wouldn't be a big deal in a compiled language but this has some overhead in php I'm not sure is useful.

If I remember correctly in php, they force the interpreter to process all these string constants even if they aren't used. That's not great.

What are the point of these. They look very excessive unless they're for external module interaction. Even then, I don't see a other contrib module "userpoints_trans_ucpoints" where calling it as a
constant would be more useful. A change in API is a change in API.

If this patch stems from morbus, I'm guessing "admin userpoints" vs "administer userpoints" is also an issue on the minds of visitors to this issue.

Actions 2.x had a need for an updated permissions string, so I submitted a patch which updates hook_perm() and runs a single query to update existing assigned permissions to the new string.

in a recent commit "admin userpoints" was changed to "administer userpoints". The issue of constants can be taken up at another time. Unfortunately, however, the current patch no longer applies although if there is a consensus on this issue it'll be easy to reroll it.

I'm going to postpone this because its clogging the issue queue for an issue that's more a clean-code debate than actual "help the user out" code.

Just want to add an amen. This module needs to go on a diet. :-)

Closing this as outdated, 6.x version is no longer maintained.