This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Duplicated code in decimal module
Type: enhancement Stage:
Components: Versions: Python 3.3
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: mark.dickinson Nosy List: eric.araujo, mark.dickinson, skreft
Priority: normal Keywords:

Created on 2011-11-07 05:14 by skreft, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Messages (5)
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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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.
History
Date User Action Args
2022-04-11 14:57:23adminsetgithub: 57573
2011-11-08 17:06:43mark.dickinsonsetstatus: open -> closed
type: enhancement
messages: + msg147309

assignee: mark.dickinson
resolution: rejected
2011-11-07 17:03:51eric.araujosetnosy: + eric.araujo
title: Duplicated Code -> Duplicated code in decimal module
messages: + msg147240

versions: - Python 2.7
2011-11-07 08:12:46skreftsetmessages: + msg147209
2011-11-07 07:04:13mark.dickinsonsetnosy: + mark.dickinson
messages: + msg147206
2011-11-07 05:14:44skreftcreate