| msg147201 - (view) |
Author: (skreft) |
Date: 2011-11-07 05:14 |
| By using tools like clonedigger is possible to spot some repeated code. One file that caught my attention is Lib/decimal.py. It has many portions of duplicated code. Here is an example: def logical_or(self, other, context=None): """Applies an 'or' operation between self and other's digits.""" if context is None: context = getcontext() other = _convert_other(other, raiseit=True) if not self._islogical() or not other._islogical(): return context._raise_error(InvalidOperation) # fill to context.prec (opa, opb) = self._fill_logical(context, self._int, other._int) # make the operation, and clean starting zeroes result = "".join([str(int(a)|int(b)) for a,b in zip(opa,opb)]) return _dec_from_triple(0, result.lstrip('0') or '0', 0) def logical_xor(self, other, context=None): """Applies an 'xor' operation between self and other's digits.""" if context is None: context = getcontext() other = _convert_other(other, raiseit=True) if not self._islogical() or not other._islogical(): return context._raise_error(InvalidOperation) # fill to context.prec (opa, opb) = self._fill_logical(context, self._int, other._int) # make the operation, and clean starting zeroes result = "".join([str(int(a)^int(b)) for a,b in zip(opa,opb)]) return _dec_from_triple(0, result.lstrip('0') or '0', 0) What's the posture about this? Should this be fixed? ps: Even more duplicated code is found in python 2.7. |
|
|
| msg147206 - (view) |
Author: Mark Dickinson (mark.dickinson) *  |
Date: 2011-11-07 07:04 |
| How would you suggest refactoring this? For that example, I'd prefer not to remove the repetition; as it is, the code is clean and clear. Eliminating the repetition would involve adding an extra layer of indirection, making the code in one of these functions harder to follow. |
|
|
| msg147209 - (view) |
Author: (skreft) |
Date: 2011-11-07 08:12 |
| One possible refactor would be. import operator def logical_or(self, other, context=None): return self._logical_op(other, operator.__or__, context) def logical_xor(self, other, context=None): return self._logical_op(other, operator.__xor__, context) def logical_and(self, other, context=None): return self._logical_op(other, operator.__and__, context) def _logical_op(self, other, operation, context=None): """Applies a logical operation between self and other's digits.""" if context is None: context = getcontext() other = _convert_other(other, raiseit=True) if not self._islogical() or not other._islogical(): return context._raise_error(InvalidOperation) # fill to context.prec (opa, opb) = self._fill_logical(context, self._int, other._int) # make the operation, and clean starting zeroes result = "".join([str(operation(int(a), int(b))) for a,b in zip(opa,opb)]) return _dec_from_triple(0, result.lstrip('0') or '0', 0) |
|
|
| msg147240 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2011-11-07 17:03 |
| Thanks for the report. If I may offer recommendations about submitting bugs: - Know that stable branches don’t get code cleanups, only bug fixes, so you have to target 3.3 - Focused bugs (“code duplication in packaging commands”) are much better that over-broad bugs |
|
|
| msg147309 - (view) |
Author: Mark Dickinson (mark.dickinson) * |
Date: 2011-11-08 17:06 |
| Sorry, I'm closing this as rejected. It might possibly have been better to write the code this way to begin with, but the code is already there, it's working, and there's little to be gained by 'fixing' it at this point. |
|
|