design criticism

Discussion in 'C++' started by yurec, Nov 30, 2007.

  1. yurec

    yurec Guest

    class BigClass
    {
    ....
    protected:
    void
    _FindLayoutToSet(CApplicationContextManager::EApplicationContext/
    *resolve_state_param*/ i_param );
    void _SetLayout(MLayoutManager::ELayoutType/*state*/ i_param );
    };

    void
    BigClass::_FindLayoutToSet(CApplicationContextManager::EApplicationContext
    i_param )
    {
    MLayoutManager::ELayoutType state_to_set;
    switch(i_param)
    {
    case 0:
    {
    A a;
    a.DoSmth();
    bool a_ret = a.GetSmth();
    B b;
    bool b_ret = b.DoSmth();
    if (a && b)
    {
    state_to_set = some_state;
    }
    else
    {
    state_to_set = another_state;
    }
    break;
    }
    case 1:
    ... //the same complex logic
    break;
    default:
    break;
    }
    _SetLayout(state_to_set);
    }

    I decided to redesign it into smth like this :


    class BigClass //more 15000 lines in cpp
    {
    ....
    public:
    typedef boost::function<MLayoutManager::ELayoutType (void)>
    get_layout_func;
    typedef boost::shared_ptr<get_layout_func> p_get_layout_func;

    void
    SetLayoutFuncToContextPredicate(boost::shared_ptr<IContextToLayoutPred>
    i_pred);
    boost::shared_ptr<IContextToLayoutPred>
    GetLayoutFuncToContextPredicate() const;

    void
    SetLayoutFuncToContext(CApplicationContextManager::EApplicationContext
    i_context,
    const std::string_t & i_name_origin,
    p_get_layout_func ip_get_layout_type_from_context);

    bool
    RemoveLayoutFuncToContext(CApplicationContextManager::EApplicationContext
    i_context,
    const std::string_t & i_name_origin);

    p_get_layout_func
    GetLayoutFuncToContext(CApplicationContextManager::EApplicationContext
    i_context,
    const std::string_t & i_name_origin) const;

    protected:
    void _FindApplicationStateToSet(int i_param );
    void _SetApplicationState(state i_param );

    private:
    struct CSCDocImpl;
    std::auto_ptr<CSCDocImpl> mp_impl;
    };



    class DefaultContextToLayoutPred:public IContextToLayoutPred
    {
    public:

    DefaultContextToLayoutPred():m_layout_type(MLayoutManager::LAYOUT_UNKNOWN)
    {};

    virtual MLayoutManager::ELayoutType GetLayoutType() const
    {
    return m_layout_type;
    }

    bool Compare(CSCDoc::p_get_layout_func i_func) const // bad
    method name. means get result of function ,
    // change
    internal predicate state and return,
    // if got
    enough information
    {
    const MLayoutManager::ELayoutType layout_type = (*i_func)();
    if (layout_type != m_layout_type)
    {
    m_layout_type = layout_type;
    return true;
    }
    return false;
    }

    virtual void Reset()
    {
    m_layout_type = MLayoutManager::LAYOUT_UNKNOWN;
    }

    private:
    mutable MLayoutManager::ELayoutType m_layout_type;
    };

    //-----------------------------------------------------------------------------
    struct CSCDoc::CSCDocImpl
    {
    ContextToLayout m_context_to_layout;
    boost::shared_ptr<IContextToLayoutPred> mp_layout_to_contex_pred;

    MLayoutManager::ELayoutType
    GetLayoutType(CApplicationContextManager::EApplicationContext
    i_context) const;
    bool
    RemoveLayoutFuncToContext(CApplicationContextManager::EApplicationContext
    i_context, const std::string_t & i_name_origin);
    };


    //-----------------------------------------------------------------------------
    MLayoutManager::ELayoutType
    CSCDoc::CSCDocImpl::GetLayoutType(CApplicationContextManager::EApplicationContext
    i_context) const
    {
    if (false == mp_layout_to_contex_pred)
    {
    return MLayoutManager::LAYOUT_UNKNOWN;
    }

    return m_context_to_layout.GetLayoutType(i_context,
    mp_layout_to_contex_pred);
    }

    //-----------------------------------------------------------------------------
    bool
    CSCDoc::CSCDocImpl::RemoveLayoutFuncToContext(CApplicationContextManager::EApplicationContext
    i_context, const std::string_t & i_name_origin)
    {
    bool ret =
    m_context_to_layout.RemoveLayoutFuncToContext(i_context,
    i_name_origin);
    return ret;
    }



    //------------------------------------------------------------------------------------
    void
    CSCDoc::SetLayoutFuncToContext(CApplicationContextManager::EApplicationContext
    i_context,
    const std::string_t & i_name_origin,
    p_get_layout_func ip_get_layout_type_from_context)
    {
    mp_impl->m_context_to_layout.SetLayoutFuncToContext(i_context,
    i_name_origin, ip_get_layout_type_from_context);
    }

    //------------------------------------------------------------------------------------
    bool
    CSCDoc::RemoveLayoutFuncToContext(CApplicationContextManager::EApplicationContext
    i_context,
    const std::string_t & i_name_origin)
    {
    return mp_impl->RemoveLayoutFuncToContext(i_context,
    i_name_origin);
    }

    //------------------------------------------------------------------------------------
    CSCDoc::p_get_layout_func
    CSCDoc::GetLayoutFuncToContext(CApplicationContextManager::EApplicationContext
    i_context,
    const std::string_t & i_name_origin) const
    {
    return mp_impl-
    >m_context_to_layout.GetLayoutFuncToContext(i_context, i_name_origin);

    }




    //------------------------------------------------------------------------------------
    void
    CSCDoc::SetLayoutFuncToContextPredicate(boost::shared_ptr<IContextToLayoutPred>
    i_pred)
    {
    mp_impl->mp_layout_to_contex_pred = i_pred;
    }

    //------------------------------------------------------------------------------------
    boost::shared_ptr<IContextToLayoutPred>
    CSCDoc::GetLayoutFuncToContextPredicate() const
    {
    return mp_impl->mp_layout_to_contex_pred;
    }


    /*-------------------ContextToLayout + IContextToLayoutPred
    ---------------------------------------*/
    #pragma once

    #include <boost/function.hpp>
    #include "MLayoutManager.h"

    class IContextToLayoutPred
    {
    public:
    virtual ~IContextToLayoutPred(){};
    virtual void Reset(){};
    virtual MLayoutManager::ELayoutType GetLayoutType() const
    {return MLayoutManager::LAYOUT_UNKNOWN;};
    virtual bool Compare(CSCDoc::p_get_layout_func i_func) const
    { return false;};

    bool operator()(const CSCDoc::p_get_layout_func & i_func)
    {
    return Compare(i_func);
    }
    };

    //-----------------------------------------------------------------------------
    class ContextToLayout
    {
    public:
    typedef boost::function<MLayoutManager::ELayoutType (void)>
    get_layout_func;
    typedef boost::shared_ptr<get_layout_func> p_get_layout_func;

    private:
    typedef std::map<std::string_t, p_get_layout_func >
    get_layout_func_map;
    typedef
    std::map<CApplicationContextManager::EApplicationContext,
    get_layout_func_map > context_to_layout;

    public:
    MLayoutManager::ELayoutType
    GetLayoutType(CApplicationContextManager::EApplicationContext
    i_context,
    boost::shared_ptr<IContextToLayoutPred> ip_pred) const;

    p_get_layout_func
    GetLayoutFuncToContext(CApplicationContextManager::EApplicationContext
    i_context,
    const std::string_t & i_name_origin) const;

    void
    SetLayoutFuncToContext(CApplicationContextManager::EApplicationContext
    i_context,
    const std::string_t & i_name_origin,
    p_get_layout_func i_get_layout_type_from_context);

    bool
    RemoveLayoutFuncToContext(CApplicationContextManager::EApplicationContext
    i_context,
    const std::string_t & i_name_origin);

    private:
    context_to_layout m_context_to_layout;
    };

    //-----------------------------------------------------------------------------
    #include "stdafx.h"
    #include "ContextToLayout.h"

    #include "ApplicationContextManager.h"

    #include <boost/bind.hpp>

    //-----------------------------------------------------------------------------
    void
    ContextToLayout::SetLayoutFuncToContext( CApplicationContextManager::EApplicationContext
    i_context,
    const std::string_t & i_name_origin,
    p_get_layout_func ip_get_layout_type_from_context)
    {
    m_context_to_layout[i_context][i_name_origin] =
    ip_get_layout_type_from_context;
    }

    //-----------------------------------------------------------------------------
    bool
    ContextToLayout::RemoveLayoutFuncToContext(CApplicationContextManager::EApplicationContext
    i_context,
    const std::string_t & i_name_origin)
    {
    context_to_layout::iterator iter_context =
    std::find_if( m_context_to_layout.begin(),

    m_context_to_layout.end(),

    boost::bind(&context_to_layout::value_type::first,_1) == i_context);

    get_layout_func_map & named_func_map = iter_context->second;
    context_to_layout::mapped_type::iterator iter_context_name =
    named_func_map.find(i_name_origin);
    if (named_func_map.end() != iter_context_name)
    {
    named_func_map.erase(iter_context_name);
    return true;
    }
    return false;
    }

    //-----------------------------------------------------------------------------
    ContextToLayout::p_get_layout_func
    ContextToLayout::GetLayoutFuncToContext(CApplicationContextManager::EApplicationContext
    i_context,
    const std::string_t & i_name_origin) const
    {
    context_to_layout::const_iterator iter_context =
    std::find_if( m_context_to_layout.begin(),

    m_context_to_layout.end(),

    boost::bind(&context_to_layout::value_type::first,_1) == i_context);
    const get_layout_func_map & named_func_map = iter_context-
    >second;

    context_to_layout::mapped_type::const_iterator iter_context_name
    = named_func_map.find(i_name_origin);
    return iter_context_name != named_func_map.end() ?
    (iter_context_name->second) : p_get_layout_func();
    }

    //-----------------------------------------------------------------------------
    MLayoutManager::ELayoutType
    ContextToLayout::GetLayoutType(CApplicationContextManager::EApplicationContext
    i_context,
    boost::shared_ptr<IContextToLayoutPred> ip_pred) const
    {
    MLayoutManager::ELayoutType layout_type =
    MLayoutManager::LAYOUT_UNKNOWN;

    context_to_layout::const_iterator iter_context =
    m_context_to_layout.find(i_context);
    if (m_context_to_layout.end() == iter_context)
    {
    layout_type = ip_pred->GetLayoutType();
    ip_pred->Reset();
    return layout_type;
    }

    const get_layout_func_map & named_func_map = iter_context-
    >second;

    get_layout_func_map::const_iterator iter_func =
    std::find_if(named_func_map.begin(), named_func_map.end(),

    boost::bind(&IContextToLayoutPred::Compare, ip_pred,

    boost::bind(&get_layout_func_map::value_type::second, _1)));

    layout_type = ip_pred->GetLayoutType();
    ip_pred->Reset();
    return layout_type;
    }

    //-----------------------------------------------------------------------------


    So as a result we have :

    void
    BigClass::_FindLayoutToSet(CApplicationContextManager::EApplicationContext
    i_param )
    {
    MLayoutManager::ELayoutType state_to_set = mp_impl-
    >GetLayoutType(i_context);

    _SetLayout(state_to_set);
    }

    So there are no dependecies between this class and
    and classes wich really knows what layout we must
    have based on their internal state.

    Thank you very much if you have read all above.
    Looking forward to your comments.
     
    yurec, Nov 30, 2007
    #1
    1. Advertising

  2. yurec

    Noah Roberts Guest

    Learn about code smells and you should be able to evaluate it yourself.
     
    Noah Roberts, Nov 30, 2007
    #2
    1. Advertising

  3. yurec

    yurec Guest

    On Nov 30, 6:13 pm, Noah Roberts <> wrote:
    > Learn about code smells and you should be able to evaluate it yourself.


    Sorry for my poor english, but I can understand what do you mean
     
    yurec, Nov 30, 2007
    #3
  4. * yurec:
    > On Nov 30, 6:13 pm, Noah Roberts <> wrote:
    >> Learn about code smells and you should be able to evaluate it yourself.

    >
    > Sorry for my poor english, but I can understand what do you mean


    He means that

    class BigClass //more 15000 lines in cpp

    stinks.

    It's so rotten that you should not only think about refactoring, but
    really think about re-implementing the entire program from scratch, and
    perhaps also, at a higher level, whether that program or something like
    it is really a good solution to the information processing needs.

    Cheers, & hth.,

    - Alf

    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
     
    Alf P. Steinbach, Nov 30, 2007
    #4
  5. yurec

    Noah Roberts Guest

    Alf P. Steinbach wrote:
    > * yurec:
    >> On Nov 30, 6:13 pm, Noah Roberts <> wrote:
    >>> Learn about code smells and you should be able to evaluate it yourself.

    >>
    >> Sorry for my poor english, but I can understand what do you mean

    >
    > He means that
    >
    > class BigClass //more 15000 lines in cpp
    >
    > stinks.
    >
    > It's so rotten that you should not only think about refactoring, but
    > really think about re-implementing the entire program from scratch, and
    > perhaps also, at a higher level, whether that program or something like
    > it is really a good solution to the information processing needs.


    Sigh...that is not at all what I mean.

    Go look up "code smell" and read about the various different kinds and
    the design problems they exhibit. Then you'll have some tools with
    which to evaluate design.
     
    Noah Roberts, Nov 30, 2007
    #5
  6. * Noah Roberts:
    > Alf P. Steinbach wrote:
    >> * yurec:
    >>> On Nov 30, 6:13 pm, Noah Roberts <> wrote:
    >>>> Learn about code smells and you should be able to evaluate it yourself.
    >>>
    >>> Sorry for my poor english, but I can understand what do you mean

    >>
    >> He means that
    >>
    >> class BigClass //more 15000 lines in cpp
    >>
    >> stinks.
    >>
    >> It's so rotten that you should not only think about refactoring, but
    >> really think about re-implementing the entire program from scratch,
    >> and perhaps also, at a higher level, whether that program or something
    >> like it is really a good solution to the information processing needs.

    >
    > Sigh...that is not at all what I mean.


    Oh, sorry.

    That is what you /should/ have meant. :)


    > Go look up "code smell" and read about the various different kinds and
    > the design problems they exhibit. Then you'll have some tools with
    > which to evaluate design.


    If you don't smell the above, then your nasal cavities are probably
    clogged by demons. :D

    Cheers,

    - Alf

    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
     
    Alf P. Steinbach, Nov 30, 2007
    #6
  7. yurec

    yurec Guest

    On Dec 1, 12:00 am, "Alf P. Steinbach" <> wrote:
    > * Noah Roberts:
    >
    >
    >
    >
    >
    > > Alf P. Steinbach wrote:
    > >> * yurec:
    > >>> On Nov 30, 6:13 pm, Noah Roberts <> wrote:
    > >>>> Learn about code smells and you should be able to evaluate it yourself.

    >
    > >>> Sorry for my poor english, but I can understand what do you mean

    >
    > >> He means that

    >
    > >> class BigClass //more 15000 lines in cpp

    >
    > >> stinks.

    >
    > >> It's so rotten that you should not only think about refactoring, but
    > >> really think about re-implementing the entire program from scratch,
    > >> and perhaps also, at a higher level, whether that program or something
    > >> like it is really a good solution to the information processing needs.

    >
    > > Sigh...that is not at all what I mean.

    >
    > Oh, sorry.
    >
    > That is what you /should/ have meant. :)
    >
    > > Go look up "code smell" and read about the various different kinds and
    > > the design problems they exhibit. Then you'll have some tools with
    > > which to evaluate design.

    >
    > If you don't smell the above, then your nasal cavities are probably
    > clogged by demons. :D
    >
    > Cheers,
    >
    > - Alf
    >
    > --
    > A: Because it messes up the order in which people normally read text.
    > Q: Why is it such a bad thing?
    > A: Top-posting.
    > Q: What is the most annoying thing on usenet and in e-mail?- Hide quoted text -
    >
    > - Show quoted text -


    Wikipedia gives nice definition of "code smell" )
    However this class is a part of framework,
    maybe everyone knows that it needs refactoring (re-implementing),
    but there is no time for this given by management.
    So my question covers just a piece of possible refactoring and

    i wonder if my design decision is good ( correct, useful)
     
    yurec, Dec 1, 2007
    #7
  8. * yurec:
    > On Dec 1, 12:00 am, "Alf P. Steinbach" <> wrote:
    >> * Noah Roberts:
    >>> * Alf P. Steinbach wrote:
    >>>> * yurec:
    >>>>> On Nov 30, 6:13 pm, Noah Roberts <> wrote:
    >>>>>> Learn about code smells and you should be able to evaluate it yourself.
    >>>>> Sorry for my poor english, but I can understand what do you mean
    >>>> He means that
    >>>> class BigClass //more 15000 lines in cpp
    >>>> stinks.
    >>>> It's so rotten that you should not only think about refactoring, but
    >>>> really think about re-implementing the entire program from scratch,
    >>>> and perhaps also, at a higher level, whether that program or something
    >>>> like it is really a good solution to the information processing needs.
    >>> Sigh...that is not at all what I mean.

    >> Oh, sorry.
    >>
    >> That is what you /should/ have meant. :)
    >>
    >>> Go look up "code smell" and read about the various different kinds and
    >>> the design problems they exhibit. Then you'll have some tools with
    >>> which to evaluate design.

    >> If you don't smell the above, then your nasal cavities are probably
    >> clogged by demons. :D

    >
    > Wikipedia gives nice definition of "code smell" )


    Look up "God class".


    > However this class is a part of framework,


    Then don't touch it.


    > maybe everyone knows that it needs refactoring (re-implementing),
    > but there is no time for this given by management.
    > So my question covers just a piece of possible refactoring and
    > i wonder if my design decision is good ( correct, useful)


    It seems you have introduced additional complexity in order to remove a
    little file dependency.

    That's probably not good, and wrapping a functor in a shared_ptr, well,
    it absolutely doesn't smell right, and with the quality of original code
    shown it's probably absolutely NOT a good idea to use newfangled things
    such as Boost functions and template algorithms: think maintainance.

    But I don't have time to delve into the particulars of your solution,
    especially since you fail to explain what the heck it is meant to be a
    solution of, but regarding the original code, here are some comments:

    * "protected:" is used for internal functions that can't possibly be
    meant to be called by derived classes.

    * "_FindLayoutToSet", names starting with underscore followed by
    uppercase are reserved to the implementation. UB right here.

    * "_FindLayoutToSet" calling "_SetLayout", misleading name for the
    first, it doesn't find a layout, it sets the layout, plus,
    procedural coding.

    * Big switch and if/else statement, failure to use OO or any
    abstraction, look for bugs here.

    * Magic numbers (0, 1 and so forth).

    But all this means is that the code is a total mess and probably bugsy
    as hell, and we /knew/ that already from the God class property.

    If I were to do anything with that code (I'd never!), I'd start by
    /naming/ things, those magic numbers. In order to bring clarity. So as
    to remove the zillion bugs that are certain to lurk in there.

    Because you can't refactor something that's full of bugs unless you have
    a detailed specification of what it should do, which given the little
    original code you presented it is very improbable that you have.

    Cheers, & hth.,

    - Alf


    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
     
    Alf P. Steinbach, Dec 1, 2007
    #8
  9. yurec

    yurec Guest

    On Dec 1, 5:09 pm, "Alf P. Steinbach" <> wrote:
    > * yurec:
    >
    >
    >
    >
    >
    > > On Dec 1, 12:00 am, "Alf P. Steinbach" <> wrote:
    > >> * Noah Roberts:
    > >>> * Alf P. Steinbach wrote:
    > >>>> * yurec:
    > >>>>> On Nov 30, 6:13 pm, Noah Roberts <> wrote:
    > >>>>>> Learn about code smells and you should be able to evaluate it yourself.
    > >>>>> Sorry for my poor english, but I can understand what do you mean
    > >>>> He means that
    > >>>> class BigClass //more 15000 lines in cpp
    > >>>> stinks.
    > >>>> It's so rotten that you should not only think about refactoring, but
    > >>>> really think about re-implementing the entire program from scratch,
    > >>>> and perhaps also, at a higher level, whether that program or something
    > >>>> like it is really a good solution to the information processing needs.
    > >>> Sigh...that is not at all what I mean.
    > >> Oh, sorry.

    >
    > >> That is what you /should/ have meant. :)

    >
    > >>> Go look up "code smell" and read about the various different kinds and
    > >>> the design problems they exhibit. Then you'll have some tools with
    > >>> which to evaluate design.
    > >> If you don't smell the above, then your nasal cavities are probably
    > >> clogged by demons. :D

    >
    > > Wikipedia gives nice definition of "code smell" )

    >
    > Look up "God class".
    >
    > > However this class is a part of framework,

    >
    > Then don't touch it.
    >
    > > maybe everyone knows that it needs refactoring (re-implementing),
    > > but there is no time for this given by management.
    > > So my question covers just a piece of possible refactoring and
    > > i wonder if my design decision is good ( correct, useful)

    >
    > It seems you have introduced additional complexity in order to remove a
    > little file dependency.
    >
    > That's probably not good, and wrapping a functor in a shared_ptr, well,
    > it absolutely doesn't smell right, and with the quality of original code
    > shown it's probably absolutely NOT a good idea to use newfangled things
    > such as Boost functions and template algorithms: think maintainance.
    >
    > But I don't have time to delve into the particulars of your solution,
    > especially since you fail to explain what the heck it is meant to be a
    > solution of, but regarding the original code, here are some comments:
    >
    > * "protected:" is used for internal functions that can't possibly be
    > meant to be called by derived classes.
    >
    > * "_FindLayoutToSet", names starting with underscore followed by
    > uppercase are reserved to the implementation. UB right here.
    >
    > * "_FindLayoutToSet" calling "_SetLayout", misleading name for the
    > first, it doesn't find a layout, it sets the layout, plus,
    > procedural coding.
    >
    > * Big switch and if/else statement, failure to use OO or any
    > abstraction, look for bugs here.
    >
    > * Magic numbers (0, 1 and so forth).
    >
    > But all this means is that the code is a total mess and probably bugsy
    > as hell, and we /knew/ that already from the God class property.
    >
    > If I were to do anything with that code (I'd never!), I'd start by
    > /naming/ things, those magic numbers. In order to bring clarity. So as
    > to remove the zillion bugs that are certain to lurk in there.
    >
    > Because you can't refactor something that's full of bugs unless you have
    > a detailed specification of what it should do, which given the little
    > original code you presented it is very improbable that you have.
    >
    > Cheers, & hth.,
    >
    > - Alf
    >
    > --
    > A: Because it messes up the order in which people normally read text.
    > Q: Why is it such a bad thing?
    > A: Top-posting.
    > Q: What is the most annoying thing on usenet and in e-mail?- Hide quoted text -
    >
    > - Show quoted text -


    The piece of code i want to refactot does :
    it takes some enumed value as input parameter, then
    it examines application state using the case corresponding to the
    value, then
    it sets application view state using some value, calculated in one of
    switch statement code.

    1)There are no magics numbers ( I just didn't want to write our emuns
    as they didn't important hear)

    2)Underscore before protected methods names is a part of our coding
    conventions

    3)The target of my design is to remove switch statement and dependency
    incapsulating case logic into
    smth like Command.
    ( If it requires not a few classes and using switch it will definetly
    require more and more,
    so I don't think, that it's really little dependency).

    4)I didn't want to store those functors ( commands ) as far as they
    could be state functors and include some objects.
    So using boost::shared_ptr seemed to be simple and obvious solution.
    (as auto_ptr must not be used with containers)
     
    yurec, Dec 3, 2007
    #9
  10. On 1 Dec, 15:09, "Alf P. Steinbach" <> wrote:

    <snip>

    > But I don't have time to delve into the particulars of your solution,
    > especially since you fail to explain what the heck it is meant to be a
    > solution of, but regarding the original code, here are some comments:
    >
    > * "protected:" is used for internal functions that can't possibly be
    > meant to be called by derived classes.


    ?

    so why not make them private?

    <snip>

    --
    Nick Keighley
     
    Nick Keighley, Dec 3, 2007
    #10
  11. * yurec:
    >
    > 2)Underscore [followed by uppercase] before protected methods
    > names is a part of our coding conventions


    Usually coding conventions are made by the best and most experienced
    people in the firm.

    What you're saying is that in your firm, those people don't even know
    the basic rules of the language.

    Good luck to you! :)


    Cheers,

    - Alf

    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
     
    Alf P. Steinbach, Dec 3, 2007
    #11
  12. * Nick Keighley:
    > On 1 Dec, 15:09, "Alf P. Steinbach" <> wrote:
    >
    > <snip>
    >
    >> But I don't have time to delve into the particulars of your solution,
    >> especially since you fail to explain what the heck it is meant to be a
    >> solution of, but regarding the original code, here are some comments:
    >>
    >> * "protected:" is used for internal functions that can't possibly be
    >> meant to be called by derived classes.

    >
    > ?
    >
    > so why not make them private?


    I'm sure you didn't read what you replied to.

    It's often a good idea to read what one replies to.

    Cheers,

    - Alf

    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
     
    Alf P. Steinbach, Dec 3, 2007
    #12
  13. yurec

    mike3 Guest

    On Nov 30, 10:18 am, "Alf P. Steinbach" <> wrote:
    > * yurec:
    >
    > > On Nov 30, 6:13 pm, Noah Roberts <> wrote:
    > >> Learn about code smells and you should be able to evaluate it yourself.

    >
    > > Sorry for my poor english, but I can understand what do you mean

    >
    > He means that
    >
    > class BigClass //more 15000 lines in cpp
    >
    > stinks.
    >
    > It's so rotten that you should not only think about refactoring, but
    > really think about re-implementing the entire program from scratch, and
    > perhaps also, at a higher level, whether that program or something like
    > it is really a good solution to the information processing needs.
    >


    Does this mean it would actually be less work for him to rewrite
    entirely
    from scratch than to try and repair it?
     
    mike3, Dec 3, 2007
    #13
  14. * mike3:
    > On Nov 30, 10:18 am, "Alf P. Steinbach" <> wrote:
    >> * yurec:
    >>
    >>> On Nov 30, 6:13 pm, Noah Roberts <> wrote:
    >>>> Learn about code smells and you should be able to evaluate it yourself.
    >>> Sorry for my poor english, but I can understand what do you mean

    >> He means that
    >>
    >> class BigClass //more 15000 lines in cpp
    >>
    >> stinks.
    >>
    >> It's so rotten that you should not only think about refactoring, but
    >> really think about re-implementing the entire program from scratch, and
    >> perhaps also, at a higher level, whether that program or something like
    >> it is really a good solution to the information processing needs.
    >>

    >
    > Does this mean it would actually be less work for him to rewrite
    > entirely
    > from scratch than to try and repair it?


    No.

    It means trying to repair it and then trying to repair it and then and
    so on, is in total a lot more work, with the same sorry mess as end
    result technically. Meantime, clients may get more and more frustrated
    (of course, this doesn't apply in defense industry where you can sell
    'em tanks made of cardboard or, as in US, e.g. bullet proof vests
    guaranteed to not stop bullets). So it's a matter of perspective, what
    context you think within: thinking within only the smallest possible
    context a goto is a good solution to any execution control problem, but
    then later, in a little wider context, those gotos become problematic.

    "Higher level" means management at the proper level needs to take a
    closer look, but for that, they need good information -- and that's
    starting to get off-topic here...

    Cheers, & hth.,

    - Alf

    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
     
    Alf P. Steinbach, Dec 3, 2007
    #14
  15. yurec

    yurec Guest

    On Dec 3, 5:40 pm, "Alf P. Steinbach" <> wrote:
    > * Nick Keighley:
    >
    > > On 1 Dec, 15:09, "Alf P. Steinbach" <> wrote:

    >
    > > <snip>

    >
    > >> But I don't have time to delve into the particulars of your solution,
    > >> especially since you fail to explain what the heck it is meant to be a
    > >> solution of, but regarding the original code, here are some comments:

    >
    > >> * "protected:" is used for internal functions that can't possibly be
    > >> meant to be called by derived classes.

    >
    > > ?

    >
    > > so why not make them private?

    >
    > I'm sure you didn't read what you replied to.
    >
    > It's often a good idea to read what one replies to.
    >
    > Cheers,
    >
    > - Alf
    >
    > --
    > A: Because it messes up the order in which people normally read text.
    > Q: Why is it such a bad thing?
    > A: Top-posting.
    > Q: What is the most annoying thing on usenet and in e-mail?


    17.4.3.1.2 Global Names
    1 Certain sets of names and functions signature are always reserved
    to
    the implementation:
    -- Each name that contains a double underscore (_ _) or begins with
    an
    underscore followed by an uppercase letter (2.11) is reserved to the
    implementation for any use.
    -- Each name that begins with an underscore is reserved to the
    implementation for use as a name in the global namespace.

    Does this mean that SomeClass::Method or SomeClass::member must not
    begin from underscore followed by uppercase letter?
    I thought that it make sense only for functions in global
    namespace.But in this case
    what "-- Each name that begins with an underscore is reserved to the
    implementation for use as a name in the global namespace." stands for?
    Ok, it seems you are right.
    So i'm ashamed.
     
    yurec, Dec 4, 2007
    #15
  16. yurec

    mike3 Guest

    On Dec 3, 2:17 pm, "Alf P. Steinbach" <> wrote:
    > * mike3:
    >
    >
    >
    >
    >
    > > On Nov 30, 10:18 am, "Alf P. Steinbach" <> wrote:
    > >> * yurec:

    >
    > >>> On Nov 30, 6:13 pm, Noah Roberts <> wrote:
    > >>>> Learn about code smells and you should be able to evaluate it yourself.
    > >>> Sorry for my poor english, but I can understand what do you mean
    > >> He means that

    >
    > >> class BigClass//more 15000 lines in cpp

    >
    > >> stinks.

    >
    > >> It's so rotten that you should not only think about refactoring, but
    > >> really think about re-implementing the entire program from scratch, and
    > >> perhaps also, at a higher level, whether that program or something like
    > >> it is really a good solution to the information processing needs.

    >
    > > Does this mean it would actually be less work for him to rewrite
    > > entirely
    > > from scratch than to try and repair it?

    >
    > No.
    >


    No? Down here it sounds like you're suggesting yes, it would
    be less work to rewrite from scratch -- you say "trying to
    repair it again and again and again is in total a lot _more_
    work", not less:

    > It means trying to repair it and then trying to repair it and then and
    > so on, is in total a lot more work, with the same sorry mess as end
    > result technically. Meantime, clients may get more and more frustrated
    > (of course, this doesn't apply in defense industry where you can sell
    > 'em tanks made of cardboard or, as in US, e.g. bullet proof vests
    > guaranteed to not stop bullets). So it's a matter of perspective, what
    > context you think within: thinking within only the smallest possible
    > context a goto is a good solution to any execution control problem, but
    > then later, in a little wider context, those gotos become problematic.
    >
    > "Higher level" means management at the proper level needs to take a
    > closer look, but for that, they need good information -- and that's
    > starting to get off-topic here...
    >
     
    mike3, Dec 4, 2007
    #16
  17. yurec

    mike3 Guest

    On Dec 3, 2:17 pm, "Alf P. Steinbach" <> wrote:
    > * mike3:
    >
    >
    >
    >
    >
    > > On Nov 30, 10:18 am, "Alf P. Steinbach" <> wrote:
    > >> * yurec:

    >
    > >>> On Nov 30, 6:13 pm, Noah Roberts <> wrote:
    > >>>> Learn about code smells and you should be able to evaluate it yourself.
    > >>> Sorry for my poor english, but I can understand what do you mean
    > >> He means that

    >
    > >> class BigClass//more 15000 lines in cpp

    >
    > >> stinks.

    >
    > >> It's so rotten that you should not only think about refactoring, but
    > >> really think about re-implementing the entire program from scratch, and
    > >> perhaps also, at a higher level, whether that program or something like
    > >> it is really a good solution to the information processing needs.

    >
    > > Does this mean it would actually be less work for him to rewrite
    > > entirely
    > > from scratch than to try and repair it?

    >
    > No.
    >
    > It means trying to repair it and then trying to repair it and then and
    > so on, is in total a lot more work, with the same sorry mess as end
    > result technically. Meantime, clients may get more and more frustrated
    > (of course, this doesn't apply in defense industry where you can sell
    > 'em tanks made of cardboard or, as in US, e.g. bullet proof vests
    > guaranteed to not stop bullets). So it's a matter of perspective, what
    > context you think within: thinking within only the smallest possible
    > context a goto is a good solution to any execution control problem, but
    > then later, in a little wider context, those gotos become problematic.
    >
    > "Higher level" means management at the proper level needs to take a
    > closer look, but for that, they need good information -- and that's
    > starting to get off-topic here...
    >


    Also, is the very concept of his "class BigClass" thingy that takes
    15000+ lines of C++ code a poor idea? It seems to be a giant
    catchall of some sort, a grab bag.
     
    mike3, Dec 4, 2007
    #17
  18. On 3 Dec, 15:40, "Alf P. Steinbach" <> wrote:
    > *Nick Keighley:
    >
    > > On 1 Dec, 15:09, "Alf P. Steinbach" <> wrote:

    >
    > > <snip>

    >
    > >> But I don't have time to delve into the particulars of your solution,
    > >> especially since you fail to explain what the heck it is meant to be a
    > >> solution of, but regarding the original code, here are some comments:

    >
    > >> * "protected:" is used for internal functions that can't possibly be
    > >> meant to be called by derived classes.

    >
    > > ?

    >
    > > so why not make them private?

    >
    > I'm sure you didn't read what you replied to.
    >
    > It's often a good idea to read what one replies to.


    I'm sure I'm about look an even bigger idiot than I already do but...

    you wrote:

    ""protected:" is used for internal functions that can't possibly be
    meant to be called by derived classes."

    which I read as:

    ""protected:" is used for functions that cannot be called by
    derived
    classes."

    but protected function *can* be called by derived classes...

    so is my paraphrase wrong?


    --
    Nick Keighley
     
    Nick Keighley, Dec 4, 2007
    #18
  19. * Nick Keighley:
    > On 3 Dec, 15:40, "Alf P. Steinbach" <> wrote:
    >> *Nick Keighley:
    >>
    >>> On 1 Dec, 15:09, "Alf P. Steinbach" <> wrote:
    >>> <snip>
    >>>> But I don't have time to delve into the particulars of your solution,
    >>>> especially since you fail to explain what the heck it is meant to be a
    >>>> solution of, but regarding the original code, here are some comments:
    >>>> * "protected:" is used for internal functions that can't possibly be
    >>>> meant to be called by derived classes.
    >>> ?
    >>> so why not make them private?

    >> I'm sure you didn't read what you replied to.
    >>
    >> It's often a good idea to read what one replies to.

    >
    > I'm sure I'm about look an even bigger idiot than I already do but...
    >
    > you wrote:
    >
    > ""protected:" is used for internal functions that can't possibly be
    > meant to be called by derived classes."
    >
    > which I read as:
    >
    > ""protected:" is used for functions that cannot be called by
    > derived
    > classes."
    >
    > but protected function *can* be called by derived classes...
    >
    > so is my paraphrase wrong?


    Just a misunderstanding.

    The code presented uses "protected:" for functions that really should be
    "private:".

    Cheers, & hth.,

    - Alf

    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
     
    Alf P. Steinbach, Dec 4, 2007
    #19
  20. * mike3:
    > On Dec 3, 2:17 pm, "Alf P. Steinbach" <> wrote:
    >> * mike3:
    >>
    >>
    >>
    >>
    >>
    >>> On Nov 30, 10:18 am, "Alf P. Steinbach" <> wrote:
    >>>> * yurec:
    >>>>> On Nov 30, 6:13 pm, Noah Roberts <> wrote:
    >>>>>> Learn about code smells and you should be able to evaluate it yourself.
    >>>>> Sorry for my poor english, but I can understand what do you mean
    >>>> He means that
    >>>> class BigClass//more 15000 lines in cpp
    >>>> stinks.
    >>>> It's so rotten that you should not only think about refactoring, but
    >>>> really think about re-implementing the entire program from scratch, and
    >>>> perhaps also, at a higher level, whether that program or something like
    >>>> it is really a good solution to the information processing needs.
    >>> Does this mean it would actually be less work for him to rewrite
    >>> entirely
    >>> from scratch than to try and repair it?

    >> No.
    >>
    >> It means trying to repair it and then trying to repair it and then and
    >> so on, is in total a lot more work, with the same sorry mess as end
    >> result technically. Meantime, clients may get more and more frustrated
    >> (of course, this doesn't apply in defense industry where you can sell
    >> 'em tanks made of cardboard or, as in US, e.g. bullet proof vests
    >> guaranteed to not stop bullets). So it's a matter of perspective, what
    >> context you think within: thinking within only the smallest possible
    >> context a goto is a good solution to any execution control problem, but
    >> then later, in a little wider context, those gotos become problematic.
    >>
    >> "Higher level" means management at the proper level needs to take a
    >> closer look, but for that, they need good information -- and that's
    >> starting to get off-topic here...
    >>

    >
    > Also, is the very concept of his "class BigClass" thingy that takes
    > 15000+ lines of C++ code a poor idea? It seems to be a giant
    > catchall of some sort, a grab bag.


    Yes, it's known as "God" object (or class).

    Such large classes are symptomatic of lack of design.

    A class that has evolved to take on more and more responsibilities.

    Cheers, & hth.,

    - Alf


    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
     
    Alf P. Steinbach, Dec 4, 2007
    #20
    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. a criticism of java

    , Dec 8, 2005, in forum: Java
    Replies:
    43
    Views:
    1,383
  2. Xah Lee
    Replies:
    62
    Views:
    1,648
  3. Xah Lee
    Replies:
    61
    Views:
    1,100
  4. Replies:
    0
    Views:
    421
  5. Xah Lee
    Replies:
    58
    Views:
    476
Loading...

Share This Page