Code review comment for lp:~bialix/bzr/2.4-bug-967060

Revision history for this message
John A Meinel (jameinel) wrote :

I'm pretty sure writing "cdef public int st_dev" does create some sort of
accessor function for the attribute, since it has to turn the int into a
PyInt. So it is pretty much the same to write:

property st_dev:
  def __get__: return 0

So i would slight prefer the property approach. I don't think either is
going to be a measurable impact, though.
John
=:->
On Mar 28, 2012 6:19 PM, "Alexander Belchenko" <email address hidden> wrote:

> Jelmer Vernooij пишет:
> >> Martin Packman пишет:
> >>> How about using properties returning 0 instead? This isn't a class
> designed
> >> for minimising memory usage, but as these values serve no purpose there
> seems
> >> no point storing them.
> >>
> >> Well, thinking about it more I'd like to avoid properties because they
> >> mean more code for no real benefit. To avoid duplication in explicit
> >> initialization of those 4 members to zero, I'd prefer move them into
> >> constructor (__init__ method). There is no constructor today, but we can
> >> add it.
> > Presumably the benefit in using a property would mean 4 fields less to
> store in memory per instance, and less time to spend assigning those member
> variables per stat object. I'm not sure how big the gains of that are in
> practice though.
> >
> > I don't think that sort of change should be a requirement for this fix
> to land. But if you feel like fixing that, it would be great. :)
>
> I'm not sure what is best here. I'd like to hear opinion of John Meinel.
>
> --
> All the dude wanted was his rug back
>
> https://code.launchpad.net/~bialix/bzr/2.4-bug-967060/+merge/99737
> You are reviewing the proposed merge of lp:~bialix/bzr/2.4-bug-967060 into
> lp:bzr/2.4.
>

« Back to merge proposal