On Thu, 02 Oct 2008 07:51:30 -0700, Terrence Brannon wrote:
> Hi, I would like some feedback on how you would improve the following
> program:
> http://www.bitbucket.org/metaperl/ptc_math/src/21979c65074f/payout.py
Okay, I've read over the code, and tried to guess from context what it is
supposed to do. I've added documentation (based on my guesses!) and
modified the code in a few places. I've tried not to radically change the
overall design of your code.
Some comments:
Readable is more important than concise. There's no prize for reducing
the number of lines or leaving out comments.
I'm not really sure that your Scenario code gains much by being based on
class instances. Perhaps ordinary functions would be easier to
understand? That's probably a matter of taste though.
I don't consider it a good idea for __str__ to be used for complicated
formatted results. In general, I would expect str(obj) to return a simple
string version of the object. (Note that "simple" doesn't necessarily
mean short.)
Your Rates class is simple enough that you could probably replace it with
a plain tuple:
rates = { # lookup table for standard and VIP payment rates.
'std': (1, 0.5), 'vip': (1.25, 1.25)}
And then later in the code rates[member_type].per_click would become
rates[member_type][0]. I wouldn't do that for more than two fields per
record.
An even better idea would be the NamedTuple found here:
http://code.activestate.com/recipes/500261/
which is included in Python 2.6. For your simple purposes, the class Rate
is probably good enough.
Here's my code, untested so there's no guarantee it will work. Also,
because I'm lazy I haven't tried to fix long lines that have word-
wrapped, sorry.
I make no claim of copyright on the following, feel free to use it or
discard it.
======
class Rates:
"""Record holding two fields.
The fields are payment rates in cents per click and per
referred(?) click. (FIXME : what is per_ref_click???)
"""
def __init__(self, per_click, per_ref_click):
self.per_click = per_click
self.per_ref_click = per_ref_click
def __str__(self):
return '<Record: (%.2f, %.2f)>' % (self.per_click,
self.per_ref_click)
ad_rate = 200 # 2 dollars for 100 clicks FIXME: is this actually used?
rates = { # lookup table for standard and VIP payment rates.
'std': Rates(per_click=1, per_ref_click=0.5),
'vip': Rates(per_click=1.25, per_ref_click=1.25)
}
class Scenario:
"""Scenerio is a financial What-If calculation."""
def __init__(self, std_clicks, vip_clicks, upline_status):
"""Initialise an instance with:
std_clicks = the number of standard clicks to be paid (???)
vip_clicks = the number of VIP clicks to be paid (???)
upline_status = one of None, "std" or "vip"
(but I don't know what the different statuses mean...)
"""
self.clicks = {'std': std_clicks, 'vip' = vip_clicks}
self.payout = {}
self.ad_rate = 200
self.upline_status = upline_status
self.setup_payup()
def setup_payup(self):
"""Set up the payout dictionary."""
for member_type in rates: # FIXME: should rates be global?
self.payout[member_type] = self.clicks[member_type] * rates
[member_type].per_click
if self.upline_status is None:
self.payout['upline'] = 0
else:
self.payout['upline'] = sum(self.clicks.values()) * rates
[upline_status].per_ref_click
def calc_profit(self):
"""Return the profit made."""
return self.ad_rate - sum(self.payout.values())
def display(self):
"""Pretty print a nicely formatted table of the scenario."""
profit = self.calc_profit()
template = """
Scenario: <no description>
===========================================================
Upline status: %5s Upline payout: %.1f
Std clicks: %5d Std payout: %.1f
VIP clicks: %5d VIP payout: %.1f
Profit: %.1f
"""
s = self # alias to save space
return template % (s.upline_status, s.payout['upline'],
s.clicks['std'], s.payout['std'], s.clicks['vip'],
s.payout['vip'], profit)
if __name__ == '__main__':
# Set up some scenarios.
scenarios = []
for upline_status in [None, 'std', 'vip']:
for vip_clicks in [0, 25, 50, 75, 100]:
std_clicks = 100 - vip_clicks
scenarios.append(
Scenario(std_clicks, vip_clicks, upline_status)
)
# And display them.
for s in scenarios:
print s.display()
--
http://mail.python.org/mailman/listinfo/python-list
