Lagg / steamodd

Python module for the steam API
ISC License
76 stars 18 forks source link

Add cells_used method #31

Closed juanchristian closed 10 years ago

juanchristian commented 10 years ago

Why do we need this method?

I know that we can call it by len(inventory), but in some cases like using the Flask framework, we have to pass all arguments in the render_template(), so to pass the used slots we would have to create another argument, let's say 'cells_used=len(inventory)', something that can be avoided having a property in the items module to get the length. We have cells_total, why not have this one too?

Without cells_used: return render_template('profile.html', user=_user, inventory=_inventory, cells_used=len(_inventory))

With cells_used: return render_template('profile.html', user=_user, inventory=_inventory)

rjackson commented 10 years ago

I don't think we need property methods which are doing nothing but calling native functions on the underlying data, it seems wholly redundant to me.

In Flask particularly, you can use the jinja2 length filter to run len on any list, e.g. {{ inventory|length }} is comparable to print(len(inventory)).

juanchristian commented 10 years ago

Yes, we can use the length filter, but in some cases we won't have this possibility. It's a bit redundant, but it's very practical. It's a bit strange to have a cells_total, and to get the 'cells_used' you have to call len() on the inventory object.

Lagg commented 10 years ago

cells_total is actually a getter for something returned in the API and there's no builtin that can do the same, but here you can just call len and in general with python it's encouraged to use builtins when possible for consistency and optimization. That's why certain methods exist for implementing like __len__ and __getitem__

juanchristian commented 10 years ago

So, this is an "pythonic" approach? Didn't know about that, thanks!