Duplicated Code Woes

Discussion in 'C++' started by mike3, Jun 12, 2012.

  1. mike3

    mike3 Guest

    Hi.

    I've heard that, and it seems like a pretty good rule, duplicated code
    is bad. Duplicated code is harder to maintain -- if you update one you
    better make sure you update the clones (and if you forget one --
    BUG!!!!!), harder to read, etc. But in some cases, just what can you
    do?

    Like this little puppy I've got. It is supposed to implement
    "saturation arithmetic" (i.e. where a number is "clamped" in certain
    bounds, so that when it "overflows", the result is "clipped" at the
    bounds.). I have two routines -- one to add, one to subtract -- but
    they look kinda similar: they're the same basic structure but with
    different function calls:

    ---

    SaturationNumber<T> &operator+=(const T rhs)
    {
    if(DoesAdditionExceedMax(number, rhs, max))
    number = max;
    else if(DoesAdditionExceedMin(number, rhs, min))
    number = min;
    else
    number += rhs;

    return(*this);
    }


    SaturationNumber<T> &operator-=(const T rhs)
    {
    if(DoesSubtractionExceedMax(number, rhs, max))
    number = max;
    else if(DoesSubtractionExceedMin(number, rhs, min))
    number = min;
    else
    number -= rhs;

    return(*this);
    }

    ---

    It looks kind of duplicatey. Should one be worried about this? Yet it
    seems there is no easy way to merge these since the function calls are
    thoroughly different.

    (You may wonder: why do we need a special function
    "DoesAdditionExceedMax", etc.? Why not just "if a + b > max then..."?
    The answer, of course, is that a + b may Overflow, and the routine is
    designed to catch that. To write the checks directly in the ifs would
    be a nasty complicated conditional, and so I move that to a separate
    function to keep things clear.)

    So, is this "duplicated code", as in the "bad thing" I worry about,
    and if so, is there any better way to do this that doesn't just add
    more complexity with no clear benefit? What would you do if you were
    writing those routines?
     
    mike3, Jun 12, 2012
    #1
    1. Advertising

  2. mike3

    Guest

    On Tuesday, June 12, 2012 8:51:09 AM UTC+2, mike3 wrote:
    > Hi.
    >
    > I've heard that, and it seems like a pretty good rule, duplicated code
    > is bad. Duplicated code is harder to maintain -- if you update one you
    > better make sure you update the clones (and if you forget one --
    > BUG!!!!!), harder to read, etc. But in some cases, just what can you
    > do?
    >
    > Like this little puppy I've got. It is supposed to implement
    > "saturation arithmetic" (i.e. where a number is "clamped" in certain
    > bounds, so that when it "overflows", the result is "clipped" at the
    > bounds.). I have two routines -- one to add, one to subtract -- but
    > they look kinda similar: they're the same basic structure but with
    > different function calls:
    >
    > ---
    >
    > SaturationNumber<T> &operator+=(const T rhs)
    > {
    > if(DoesAdditionExceedMax(number, rhs, max))
    > number = max;
    > else if(DoesAdditionExceedMin(number, rhs, min))
    > number = min;
    > else
    > number += rhs;
    >
    > return(*this);
    > }
    >
    >
    > SaturationNumber<T> &operator-=(const T rhs)
    > {
    > if(DoesSubtractionExceedMax(number, rhs, max))
    > number = max;
    > else if(DoesSubtractionExceedMin(number, rhs, min))
    > number = min;
    > else
    > number -= rhs;
    >
    > return(*this);
    > }
    >
    > ---
    >
    > It looks kind of duplicatey. Should one be worried about this? Yet it
    > seems there is no easy way to merge these since the function calls are
    > thoroughly different.
    >
    > (You may wonder: why do we need a special function
    > "DoesAdditionExceedMax", etc.? Why not just "if a + b > max then..."?
    > The answer, of course, is that a + b may Overflow, and the routine is
    > designed to catch that. To write the checks directly in the ifs would
    > be a nasty complicated conditional, and so I move that to a separate
    > function to keep things clear.)
    >
    > So, is this "duplicated code", as in the "bad thing" I worry about,
    > and if so, is there any better way to do this that doesn't just add
    > more complexity with no clear benefit? What would you do if you were
    > writing those routines?


    I would keep my eye on how often and how these change. If often, and in sync, then I would err towards one helper routine that does the work and parametrize moving parts.

    I would not lose sleep over this detail. Duplicated code can be MUCH worse, and there's a point of diminishing return for ANYTHING.

    Goran.
     
    , Jun 12, 2012
    #2
    1. Advertising

  3. mike3 <> wrote:
    > I've heard that, and it seems like a pretty good rule, duplicated code
    > is bad. Duplicated code is harder to maintain -- if you update one you
    > better make sure you update the clones (and if you forget one --
    > BUG!!!!!), harder to read, etc. But in some cases, just what can you
    > do?


    In general that's true (and the longer the duplicated code, the worse).
    However, there are situations where "unduplicating" it only makes things
    worse (ie. rather than having two lean&clean functions that look almost
    identical, you end up having one single function that's a mess full of
    conditionals or other extraneous code, which isn't much better).

    > SaturationNumber<T> &operator+=(const T rhs)
    > {
    > if(DoesAdditionExceedMax(number, rhs, max))
    > number = max;
    > else if(DoesAdditionExceedMin(number, rhs, min))
    > number = min;
    > else
    > number += rhs;
    >
    > return(*this);
    > }
    >
    >
    > SaturationNumber<T> &operator-=(const T rhs)
    > {
    > if(DoesSubtractionExceedMax(number, rhs, max))
    > number = max;
    > else if(DoesSubtractionExceedMin(number, rhs, min))
    > number = min;
    > else
    > number -= rhs;
    >
    > return(*this);
    > }


    Those two functions are so short that I'd say it's not worth the effort
    to try to merge them. The resulting function would most probably be almost
    as long as those two functions combined and/or ridden with conditionals
    or complicated template metaprogramming.
     
    Juha Nieminen, Jun 12, 2012
    #3
  4. I already deleted the original message so excuse me for tagging this into
    the wrong part of the tree.

    How about sidestepping the entire issue:

    SaturationNumber<T> &operator-=(const T rhs)
    {
    return(operator+=(-rhs));
    }
     
    Terry Richards, Jun 12, 2012
    #4
  5. mike3

    mike3 Guest

    On Jun 12, 7:53 am, "Terry Richards" <> wrote:
    > I already deleted the original message so excuse me for tagging this into
    > the wrong part of the tree.
    >
    > How about sidestepping the entire issue:
    >
    > SaturationNumber<T> &operator-=(const T rhs)
    >     {
    >       return(operator+=(-rhs));
    >     }


    I thought of that too, but that requires the range of the
    SaturationNumber
    to be perfectly balanced +/- - wise, i.e. -n to +n, but that might not
    be the
    case. The range can be made essentially anything. And you _couldn't_
    have a range like that if T = unsigned int, for example.
     
    mike3, Jun 12, 2012
    #5
  6. mike3

    Pavel Guest

    mike3 wrote:
    > Hi.
    >
    > I've heard that, and it seems like a pretty good rule, duplicated code
    > is bad. Duplicated code is harder to maintain -- if you update one you
    > better make sure you update the clones (and if you forget one --
    > BUG!!!!!), harder to read, etc. But in some cases, just what can you
    > do?
    >
    > Like this little puppy I've got. It is supposed to implement
    > "saturation arithmetic" (i.e. where a number is "clamped" in certain
    > bounds, so that when it "overflows", the result is "clipped" at the
    > bounds.). I have two routines -- one to add, one to subtract -- but
    > they look kinda similar: they're the same basic structure but with
    > different function calls:
    >
    > ---
    >
    > SaturationNumber<T> &operator+=(const T rhs)
    > {
    > if(DoesAdditionExceedMax(number, rhs, max))
    > number = max;
    > else if(DoesAdditionExceedMin(number, rhs, min))
    > number = min;
    > else
    > number += rhs;
    >
    > return(*this);
    > }
    >
    >
    > SaturationNumber<T> &operator-=(const T rhs)
    > {
    > if(DoesSubtractionExceedMax(number, rhs, max))
    > number = max;
    > else if(DoesSubtractionExceedMin(number, rhs, min))
    > number = min;
    > else
    > number -= rhs;
    >
    > return(*this);
    > }
    >
    > ---
    >
    > It looks kind of duplicatey. Should one be worried about this? Yet it
    > seems there is no easy way to merge these since the function calls are
    > thoroughly different.
    >
    > (You may wonder: why do we need a special function
    > "DoesAdditionExceedMax", etc.? Why not just "if a + b > max then..."?
    > The answer, of course, is that a + b may Overflow, and the routine is
    > designed to catch that. To write the checks directly in the ifs would
    > be a nasty complicated conditional, and so I move that to a separate
    > function to keep things clear.)
    >
    > So, is this "duplicated code", as in the "bad thing" I worry about,
    > and if so, is there any better way to do this that doesn't just add
    > more complexity with no clear benefit? What would you do if you were
    > writing those routines?
    >


    How are your Does...() functions implemented? Do they try to calculate the
    result in a register and check CPU flags by any chance? Then maybe you have
    over-encapsulated your algo already.

    -Pavel
     
    Pavel, Jun 13, 2012
    #6
  7. Pavel <> wrote:
    > How are your Does...() functions implemented? Do they try to calculate the
    > result in a register and check CPU flags by any chance? Then maybe you have
    > over-encapsulated your algo already.


    Did you use a bullshit generator to come up with that paragraph?
     
    Juha Nieminen, Jun 13, 2012
    #7
  8. mike3

    Pavel Guest

    Juha Nieminen wrote:
    > Pavel <> wrote:
    >> How are your Does...() functions implemented? Do they try to calculate the
    >> result in a register and check CPU flags by any chance? Then maybe you have
    >> over-encapsulated your algo already.

    >
    > Did you use a bullshit generator to come up with that paragraph?

    You surely used all your good manners to come up with yours.
     
    Pavel, Jun 14, 2012
    #8
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Tom
    Replies:
    6
    Views:
    400
  2. Replies:
    0
    Views:
    440
  3. =?Utf-8?B?SXNhYmVsIFB1aWdkZXZhbGw=?=

    Duplicated records in a report from two tables

    =?Utf-8?B?SXNhYmVsIFB1aWdkZXZhbGw=?=, Apr 4, 2005, in forum: ASP .Net
    Replies:
    1
    Views:
    374
    =?Utf-8?B?SXNhYmVsIFB1aWdkZXZhbGw=?=
    Apr 6, 2005
  4. =?Utf-8?B?QW5kcmU=?=
    Replies:
    0
    Views:
    406
    =?Utf-8?B?QW5kcmU=?=
    Oct 27, 2005
  5. Erica
    Replies:
    5
    Views:
    407
    Jon A. Cruz
    Feb 26, 2004
Loading...

Share This Page