enowars/enochecker

Deprecate self.round or self.current_round

Closed this issue · 6 comments

I'm not sure which one to keep, but I would replace the other one with:

@property
def current_round(self):
    raise DeprecationWarning(...)
    return self.round

This alias was, if I recall correctly, introduced to make the distinction between flag_round and current_round clear, for checkers that use both. Others may prefer round as it's more concise.

I don't see a (big) conceptional problem in keeping an alias around if it depends on the individual use-case?

That just seems extremely confusing to me, especially since these are technically not aliases but two variables initialized with the same value. If one of those is changed due changes in the library at some point, they are no longer guaranteed to be equal. As you mention current_round is explicit enough to distinguish between the flag_round, so it might seem favorable to deprecate round in favor of the self-explanatory current_round

I think 99.9% of the checker use round though, so get rid of current_round instead

Also 0% of the checkers change either value, so I wouldn't think it's too bad, but then again I don't have a strong opinion, either

I'm not talking about the checkers changing those values, I am talking about future changes made to the enochecker library by people who are not aware of the difference (or lack thereof) between the two variables

All I can say is, to this date, having both values around has not been an issue, but also I don't care that much. Keep it at round (and not round_id, btw., that makes no sense at all.)