设计批评 [英] design criticism
问题描述
class BigClass
{
....
protected：
void
_FindLayoutToSet（CApplicationContextManager :: EAppl icationContext /
* resolve_state_param * / i_param）;
void _SetLayout（MLayoutManager :: ELayoutType / * state * / i_param）;
};
void
BigClass :: _ FindLayoutToSet（CApplicationContextMana ger： ：EApplicationContext
i_param）
{
MLayoutManager :: ELayoutType state_to_set;
switch（i_param）
{
案例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;
}
休息;
}
案例1：
... //同样复杂的逻辑
休息;
默认：
休息;
}
_SetLayout（state_to_set）;
}
我决定将它重新设计为smth：
class BigClass //更多15000行cpp
{< br $> b $ b ....
public：
typedef boost :: function< MLayoutManager :: ELayoutType（void）>
get_layout_func;
typedef boost :: shared_ptr< get_layout_funcp_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（CApplicationContextManag er :: 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 :: au to_ptr< CSCDocImplmp_impl;
};
class DefaultContextToLayoutPred：public IContextToLayoutPred
{
public：
DefaultContextToLayoutPred（）：m_layout_type（MLayout Manager :: LAYOUT_UNKNOWN）
{};
虚拟MLayoutManager： ：ELayoutType GetLayoutType（）const
{
返回m_layout_type;
}
bool比较（ CSCDoc :: p_get_layout_func i_func）const //坏
方法名称。意味着获得功能的结果，
//更改
内部谓词状态并返回，
//如果获得
足够的信息
{
const MLayoutManager :: ELayoutType layout_type =（* i_func）（）;
if（layout_type！= m_layout_type）
{
m_layout_type = layout_type;
返回true;
}
返回false ;
}
虚拟无效重置（）
{
m_layout_type = MLayoutManager :: LAYOUT_UNKNOWN ;
}
私有：
mutable MLayoutManager :: ELayoutType m_layout_type;
};
// ------------------------------------- ----------------------------------------
struct CSCDoc :: CSCDocImpl
{
ContextToLayout m_context_to_layout;
boost :: shared_ptr< IContextToLayoutPredmp_layout_to _contex_pred;
MLayoutManager :: ELayoutType
GetLayoutType（CApplicationConte xtManager :: EApplica tionContext
i_context）const;
bool
RemoveLayoutFuncToContext（CApplicationContextManag er :: EApplicationContext
i_context ，const std :: string_t& i_name_origin）;
};
// --------------------------- --------------------------------------------------
MLayoutManager :: ELayoutType
CSCDoc :: CSCDocImpl :: GetLayoutType（CApplicationCont extManager :: EApplicationContext
i_context）const
{
if（false == mp_layout_to_contex_pred）
{
返回MLayoutManager :: LAYOUT_UNKNOWN;
}
返回m_context_to_layout.GetLayoutType（i_context，
mp_layout_to_contex_pred）;
}
// ----------------------------------------------- ------------------------------
bool
CSCDoc :: CSCDocImpl :: RemoveLayoutFuncToContext（CApp licationContextManager :: EApplicationContext
i_context，const std :: string_t& i_name_origin）
{
bool ret =
m_context_to_layout.RemoveLayoutFuncToContext（i_co ntext，
i_name_origin）;
返回ret;
}
// --------------------------------------- ---------------------------------------------
void
CSCDoc :: SetLayoutFuncToContext（CApplicationContext Manager :: 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_cont ext，
i_name_origin ，ip_get_layout_type_from_context）;
}
// ----------------------- -------------------------------------------------- -----------
bool
CSCDoc :: RemoveLayoutFuncToContext（CApplicationCont extManager :: EApplicationContext
i_context，
const std :: string_t& i_name_origin）
{
返回mp_impl-> RemoveLayoutFuncToContext（i_context，
i_name_origin ）;
}
// ------------------------- -------------------------------------------------- ---------
CSCDoc :: p_get_layout_func
CSCDoc :: GetLayoutFuncToContext（CApplicationContext Manager :: EApplicationContext
i_context，
const std :: string_t& i_name_origin）const
{
返回mp_ impl-
class BigClass
{
....
protected:
void
_FindLayoutToSet(CApplicationContextManager::EAppl icationContext/
*resolve_state_param*/ i_param );
void _SetLayout(MLayoutManager::ELayoutType/*state*/ i_param );
};
void
BigClass::_FindLayoutToSet(CApplicationContextMana ger::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_funcp_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(CApplicationContextManag er::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<CSCDocImplmp_impl;
};
class DefaultContextToLayoutPred:public IContextToLayoutPred
{
public:
DefaultContextToLayoutPred():m_layout_type(MLayout Manager::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<IContextToLayoutPredmp_layout_to _contex_pred;
MLayoutManager::ELayoutType
GetLayoutType(CApplicationContextManager::EApplica tionContext
i_context) const;
bool
RemoveLayoutFuncToContext(CApplicationContextManag er::EApplicationContext
i_context, const std::string_t & i_name_origin);
};
//-----------------------------------------------------------------------------
MLayoutManager::ELayoutType
CSCDoc::CSCDocImpl::GetLayoutType(CApplicationCont extManager::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(CApp licationContextManager::EApplicationContext
i_context, const std::string_t & i_name_origin)
{
bool ret =
m_context_to_layout.RemoveLayoutFuncToContext(i_co ntext,
i_name_origin);
return ret;
}
//------------------------------------------------------------------------------------
void
CSCDoc::SetLayoutFuncToContext(CApplicationContext Manager::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_cont ext,
i_name_origin, ip_get_layout_type_from_context);
}
//------------------------------------------------------------------------------------
bool
CSCDoc::RemoveLayoutFuncToContext(CApplicationCont extManager::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(CApplicationContext Manager::EApplicationContext
i_context,
const std::string_t & i_name_origin) const
{
return mp_impl-
> m_context_to_layout.GetLayoutFuncToContext（i_cont ext，i_name_origin）;
>m_context_to_layout.GetLayoutFuncToContext(i_cont ext, i_name_origin);
}
// --------------- -------------------------------------------------- -------------------
void
CSCDoc :: SetLayoutFuncToContextPredicate（boost :: sha red_ptr< IContextToLayoutPred>
i_pred）
{
mp_impl-> mp_layout_to_contex_pred = i_pred;
}
// ------------------------------------------- -----------------------------------------
提升:: shared_ptr< IContextToLayoutPred>
CSCDoc :: GetLayoutFuncToContextPredicate（）const
{
返回mp_impl-> mp_layout_to_contex_pred;
}
/ * ------------------- ContextToLayout + IContextToLayoutPred
------ --------------------------------- * /
#pragma曾经
#include< boost / function.hpp>
#include" MLayoutManager.h"
class IContextToLayoutPred
{
public：
virtual~IContextToLayoutPred（）{};
virtual void Reset（）{};
虚拟MLayoutManager :: ELayoutType GetLayoutType（）const
{return MLayoutManager :: LAYOUT_UNKNOWN;};
虚拟布尔比较（CSCDoc :: p_get_layout_func i_func）const
{return false;};
bool operator（）（const CSCDoc :: p_get_layout_func& i_func）
{
返回比较（i_func）;
}
};
// ------------------------------------------ -----------------------------------
class ContextToLayout
{
public：
typedef boost :: function< MLayoutManager :: ELayoutType（void）>
get_layout_func;
typedef boost :: shared_ptr< get_layout_funcp_get_layout_func;
private：
typedef std :: map< std :: string_t，p_get_layout_func>
get_layout_func_map;
typedef
std :: map< CApplicationContextManager :: EApplicationC ontext，
get_layout_func_map context_to_layout;
public：
MLayoutManager :: ELayoutType
GetLayoutType（CApplicationContextManager :: EApplica tionContext
i_context，
boost :: shared_ptr< IContextToLayoutPredip_pred）const;
p_get_layout_func
GetLayoutFuncToContext（CA pplicationContextManager :: 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（CApplicationContextManag er :: 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 :: SetLayoutFu ncToContext（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（CApplic ationContextManager :: 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（b） named_func_map.end（）！= iter_context_name）
{
named_func_map.erase（iter_context_name）;
返回true;
}
返回false;
}
// ------------- -------------------------------------------------- --------------
ContextToLayout :: p_get_layout_func
ContextToLayout :: GetLayoutFuncToContext（CApplicati onContextManager :: 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-
}
//------------------------------------------------------------------------------------
void
CSCDoc::SetLayoutFuncToContextPredicate(boost::sha red_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_funcp_get_layout_func ;
private:
typedef std::map<std::string_t, p_get_layout_func >
get_layout_func_map;
typedef
std::map<CApplicationContextManager::EApplicationC ontext,
get_layout_func_map context_to_layout;
public:
MLayoutManager::ELayoutType
GetLayoutType(CApplicationContextManager::EApplica tionContext
i_context,
boost::shared_ptr<IContextToLayoutPredip_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(CApplicationContextManag er::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(CApplic ationContextManager::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(CApplicati onContextManager::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;
>second;
context_to_layout :: mapped_type :: const_iterator iter_context_name
= named_func_map.find（i_name_origin）;
返回iter_context_name ！= named_func_map.end（）？
（iter_context_name-> second）：p_get_layout_func（）;
}
/ / ------------------------------------------------- ----------------------------
MLayoutManager :: ELayoutType
ContextToLayout： ：GetLayoutType（CApplicationContext Manager :: EApplicationContext
i_context，
boost :: shared_ptr< IContextToLayoutPredip_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->重置（）;
返回n layout_type;
}
const get_layout_func_map& named_func_map = iter_context-
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(CApplicationContext Manager::EApplicationContext
i_context,
boost::shared_ptr<IContextToLayoutPredip_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;
>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 :: seco nd ，_1）））;
layout_type = ip_pred-> GetLayoutType（）;
ip_pred->重置（）;
返回layout_type;
}
// ---------------------- -------------------------------------------------- -----
因此我们得到：
void
BigClass :: _ FindLayoutToSet（CApplicationContextMana ger :: EApplicationContext
i_param）
{
MLayoutManager :: ELayoutType state_to_set = mp_impl-
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::seco nd, _1)));
layout_type = ip_pred->GetLayoutType();
ip_pred->Reset();
return layout_type;
}
//-----------------------------------------------------------------------------
So as a result we have :
void
BigClass::_FindLayoutToSet(CApplicationContextMana ger::EApplicationContext
i_param )
{
MLayoutManager::ELayoutType state_to_set = mp_impl-
> GetLayoutType（i_context）;
>GetLayoutType(i_context);
_SetLayout（state_to_set）;
}
所以这个班级之间没有任何依赖并且
和类真的知道我们必须根据他们的内部状态确定什么布局。
非常感谢你们你已经阅读了以上所有内容。
期待你的意见。
_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.
推荐答案
了解代码味道和您应该能够自己评估。
Learn about code smells and you should be able to evaluate it yourself.
11月30日下午6:13，Noah Roberts< u ... @ example.netwrote：
On Nov 30, 6:13 pm, Noah Roberts <u...@example.netwrote:
了解代码味道，您应该能够自己评估它。
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
Alf P. Steinbach写道：
Alf P. Steinbach wrote:
* yurec：
* yurec:
> 11月30日下午6:13，Noah Roberts< ;你... @ example.netwrote：
>On Nov 30, 6:13 pm, Noah Roberts <u...@example.netwrote:
>>了解代码味道，你应该能够自己评估它。
>>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
他的意思是
类BigClass //更多15000行在cpp
很臭。
它太烂了你不应该只考虑重构，而是...... b $ b真的考虑从头开始重新实现整个程序，并且
也许，更高层次，无论是程序还是其他东西比如
它确实是一个很好的解决方案来满足信息处理需求。
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.
这篇关于设计批评的文章就介绍到这了，希望我们推荐的答案对大家有所帮助，也希望大家多多支持IT屋！
