4
\$\begingroup\$

I have a bunch of functions that return strings based upon score ranges. Then I have another function that sums everything up and assigns a numeric weight.

@property
def risk_status(self):
    """ This function assigns numeric weight to risks. It returns an over all integer score. """

    # Build a string of risks, ie "High Risk@At Risk@@Extreme Risk"
    raw_string_of_risks = self.si_risk + '@' + self.aggbs_risk + '@' + self.cfimb_risk + '@' + self.cfiab_risk + '@' + self.cei_risk

    # Build a list ["High Risk", "At Risk", "Extreme Risk"]
    risks = raw_string_of_risks.split('@')

    # Formula for over all risk status. ["High Risk", "At Risk", "Extreme Risk"] => 2+1+3 = 6
    status = risks.count("At Risk")*1 + risks.count("High Risk")*2 + risks.count("Extreme Risk")*3

    return status

I'm afraid with a gazillion records this property might slow things down. It will never get a string longer than 65 characters to parse, but will doing this over and over again really slow things down?

\$\endgroup\$
  • 1
    \$\begingroup\$ I don't know if it will speed things up a lot, but instead of building your raw_string_of_risks and then breaking it into a list, why don't you just create the list ... risks = [self.si_risk, self.aggbs_risk, ...] \$\endgroup\$ – jcfollower Apr 4 '14 at 20:48
  • \$\begingroup\$ That is much better! Slapping my forehead! \$\endgroup\$ – broinjc Apr 7 '14 at 18:26
3
\$\begingroup\$

Concatenating an splitting seems like you're making extra work for yourself. I suggest summing the results of a string-to-integer translation.

from collections import defaultdict

_RISK_SCORE = defaultdict(int, [('Extreme Risk', 3), ('High Risk', 2), ('At Risk', 1)])

@property
def risk_status(self):
    """ This function assigns numeric weight to risks. It returns an overall integer score. """
    return sum(self._RISK_SCORE[r] for r in (
        self.si_risk,
        self.aggbs_risk,
        self.cfimb_risk,
        self.cfiab_risk,
        self.cei_risk,
    ))
\$\endgroup\$
  • \$\begingroup\$ Why defaultdict? It will mask any typos. \$\endgroup\$ – Suor Apr 10 '14 at 7:49
  • \$\begingroup\$ @Suor The author never explicitly stated that self.si_risk etc. would only take on those three string values. This code, with defaultdict, most closely mimics the behaviour of the original code, which also masks typos. \$\endgroup\$ – 200_success Apr 10 '14 at 7:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.