in reply to Assigning Opsets by User Type

One obvious possible loophole is that there's no check done whether @usertypes contains duplicates. Someone who is managed to trick the system believing he's twice an "admin editor" gets the same priviledges as a "programmer".

Also, you're doing a sort values %high_type, which is a lexical sort - but you want to sort numerically. It happens to go right because ord ("-") < ord ("0"), and you don't have numbers larger than 9 involved. But it ain't "maintainable".

Some maintainance problems: the regex used to breakdown the usertypes uses the same hardcoded keys as in %attr_value. The order in which the keys are put in %attr_value is messy. There are several "configuration" thingies (content of %attr_value, the trust levels, and what they are allowed to do), scattered all over the body of the subroutine. I'd group them, and preferably, take them outside of the sub.

Also, from your introduction, it appears that a "former programmer admin" is an illegal user type. But your sub will give it a strong trust_level.

Abigail

Replies are listed 'Best First'.
Re: Re: Assigning Opsets by User Type
by djantzen (Priest) on Nov 20, 2002 at 16:54 UTC

    Thanks Abigail. @usertypes should be okay due to checks in the module from which it comes, although I'll have to audit that code to make sure. The sorting bit now does a sort { $a <=> $b }.

    The order in which the keys are put in %attr_value is messy. Well, I don't think it's messy per se, I mean it's alphabetical, but perhaps it would be clearer to group them into negative forces, neutral, and positive.

    it appears that a "former programmer admin" is an illegal user type. But your sub will give it a strong trust_level.

    This is an interesting worry. On the one hand, I've got a variety of ways to solve it, from doing another if-else to see if there's a violation of the 'former' || ('admin' || 'trainee') rule (icky), or I could make 'former' be worth a greater negative value (hackish), or do a forward lookahead in the regex I suppose (not bad). But on the other hand, these values come from UserType objects whose values are constrained by a database table, so to construct an invalid usertype would require admin privileges on the DB. And at that point the system is totally compromised anyway.

    Thanks again for the feedback!

      Why the urge to solve the parsing with a single regex? I would solve it differently. I wouldn't use %attr_value to group "tags" like 'former', 'admin' and 'trainee' together with the role. I would so something like:
      my %roles = qw /consultant -1 editor 0 programmer 2/; my %pre_attrs = qw /former -2/; my %post_attrs = qw /trainee -1 admin 1/; my $def_score = -2; my @trust = ([full => 2], [strong => 1], [normal => 0], [weak => -1], my $def_trust = 'none'; my $max_score = -10000; # Some big, negative number. foreach my $type (@usertypes) { my $score = 0; my $attrs = 0; while (my ($key, $value) = each %pre_attrs) { if ($type =~ s/\Q$key\E\s*//) { $score += $value; $saw_pre = 1; last; } } while (my ($key, $value) = each %roles) { if ($type =~ s/\Q$key\E\s*//) { $score += $value; $saw_role = 1; last; } } while (my ($key, $value) = each %post_attrs) { if ($type =~ s/\Q$key\E\s*//) { $score += $value; $saw_post = 1; last; } } if (length $type || $saw_pre && $saw_post || !$saw_role) { # Illegal or unknown usertype. $score = $def_score; } $max_score = $score if $score > $max_score; } foreach my $level (@trust) { return $level -> [0] if $max_score >= $level -> [1]; } return $def_trust;

      Abigail