缺陷：构造器做真正的工作 [英] Flaw: Constructor does Real Work

问题描述

我有一个类表示一组数字。构造函数有三个参数： startValue ， endValue 和 stepSize 。
类负责保存一个列表，其中包含开始和结束值之间的所有值，并考虑stepSize。

示例：startValue：3，endValue：1 ，stepSize = -1，Collection = {3,2,1}

我正在创建集合和一些关于构造函数中的对象的信息字符串。公共成员是只读信息字符串和集合。

我的构造函数目前执行三个操作：

• 检查参数;




• 生成代码

  信息字符串

我可以看到我的构造函数做了真正的工作，但是如何解决这个问题，解决这个问题？如果我把方法移出构造函数，它就像有init函数，并留下一个未完全初始化的对象。我的对象的存在是否可疑？或者是在构造函数中做一些工作不是很糟糕，因为仍然可以测试构造函数，因为没有创建对象引用。

对我来说看起来不对但似乎我只是找不到一个解决方案。我也考虑了建筑师，但我不知道如果这是正确的，因为你不能选择不同类型的创作。但是单元测试的责任较小。

我在C＃中编写代码，但我更喜欢一个通用的解决方案，这就是为什么文本不包含代码。

编辑：感谢编辑我的可怜的文本（：我改变了标题，因为它代表我的意见，编辑的标题没有，我不是问，真正的工作是一个缺陷，对我来说，这是参考。

http://misko.hevery.com/code-reviewers-guide/flaw-constructor-does-real-work/

解决方案

促使您保持构造函数轻量化的概念：

• 控制反转（依赖注入）


• 单一责任原则（适用于构造函数而不是类）


• 延迟初始化


• 测试


• 
  
• DRY

链接到原因的参数：

• 在构造函数中应该完成多少工作？


• 
  
• 应该是C ++构造函数做真正的工作吗？


• http://misko.hevery.com/code-reviewers-guide/flaw-constructor-does-实际工作/

如果您检查构造函数中的参数，如果这些参数无法共享验证代码从任何其他来源（setter，constructor，parameter object）

如果你填充值到集合或生成信息字符串的构造函数，代码不能

除了无法共享外，还有一种延迟，直到真正需要的时候（lazy init）。还有重叠的继承，提供更多的选项，许多方法，只做一件事，而不是一个做一切的构造函数。

你的构造函数只需要把你的类可用状态。它不必完全初始化。但是使用其他方法做真正的工作是完全自由的。这只是没有利用懒惰的初始化的想法。有时候你需要它，有时你不需要它。

只要记住，构造函数执行或调用的任何东西都会被用户/测试人员喉咙击倒。

编辑：

您还没有接受答案， ve有一些睡眠，所以我会在一个设计刺。一个好的设计是灵活的，所以我将假设它是确定，我不知道什么信息字符串，或者我们的对象是否需要通过作为一个集合来表示一组数字（因此提供了迭代器，大小），add（），remove（）等），或者只是由一个集合支持，并提供一些狭窄的专门访问这些数字（如不可变）。

这个小家伙是参数对象模式

  / **抛出异常如果符号endValue  -  startValue！= stepSize * / 
 ListDefinition（T startValue，T endValue，T stepSize）; 
  

T可以是int或long，short或char。有趣，但是一致。

  / **一个接口，独立于任何一个集合实现* / 
 ListFactory ListDefinition ld）{
 / **尽可能多的你喜欢* / 
 List< T>建立（）; 
} 
  

如果我们不需要限制对集合的访问，完成。

  / **仅提供读取访问权限。不可变如果List保持私有。 * / 
 ImmutableFacade（List l）; 
  

等待，要求改变，忘记了信息字符串。 ：）

  / **构建信息字符串列表* / 
 InformationStrings（String infoFilePath）{
 List< String>读（）; 
} 
  

不知道这是你的想法，但如果你想电源计数行数两个你现在有它。 ：）

  / **假设信息字符串与我们的数字有1到1的关系* / 
 MapFactory l，List infoStrings）{
 / **尽可能多的你喜欢* / 
映射< T，String>建立（）; 
} 
  

因此，我需要使用构建器模式将所有这些连接在一起。或者你可以尝试使用一个对象来做所有这一切。由你决定。但我认为你会发现这些构造函数很少做任何事情。

EDIT2

我知道这个答案已经被接受，但我意识到有改进的余地，我不能抗拒。上面的ListDefinition通过用getter，ick显示它的内容来工作。有一个告诉，不要问的设计原则，这是违反了这里没有好的理由。

  ListDefinition（T startValue，T endValue，T stepSize）{
 List& buildList（List< T> l）; 
} 
  

$ b $ p

这让我们构建任何类型的列表实现，定义。现在我们不需要ListFactory。 buildList是我所谓的分流器。它返回相同的引用，它接受后，做了一些事情。它只是允许你跳过给新的ArrayList一个名字。现在，列表看起来像这样：

  ListDefinition< int> ld = new ListDefinition< int>（3,1，-1）; 
 List< int> l = new ImmutableFacade< int>（ld.buildList（new ArrayList< int>（）））; 
  

位很难读。所以为什么不添加一个静态工厂方法：

  List< int& l = ImmutableRangeOfNumbers.over（3，1，-1）; 
  

这不接受依赖注入，而是基于类。它实际上是一个依赖注入容器。这使它成为流行的组合和基础类的配置的一个很好的缩写。你不必为每个组合做一个。现在你可以把你需要的组合放在一起。

好吧，这是我的2美分。我会找到别的东西来痴迷。反馈欢迎。

I have a class which represents a set of numbers. The constructor takes three arguments: startValue, endValue and stepSize. The class is responsible for holding a list containing all values between start and end value taking the stepSize into consideration.

Example: startValue: 3, endValue: 1, stepSize = -1, Collection = { 3,2,1 }

I am currently creating the collection and some info strings about the object in the constructor. The public members are read only info strings and the collection.

My constructor does three things at the moment:

• Checks the arguments; this could throw an exception from the constructor

• Fills values into the collection

• Generates the information strings

I can see that my constructor does real work but how can I fix this, or, should I fix this? If I move the "methods" out of the constructor it is like having init function and leaving me with an not fully initialized object. Is the existence of my object doubtful? Or is it not that bad to have some work done in the constructor because it is still possible to test the constructor because no object references are created.

For me it looks wrong but it seems that I just can't find a solution. I also have taken a builder into account but I am not sure if that's right because you can't choose between different types of creations. However single unit tests would have less responsibility.

I am writing my code in C# but I would prefer a general solution, that's why the text contains no code.

EDIT: Thanks for editing my poor text (: I changed the title back because it represents my opinion and the edited title did not. I am not asking if real work is a flaw or not. For me, it is. Take a look at this reference.

http://misko.hevery.com/code-reviewers-guide/flaw-constructor-does-real-work/

The blog states the problems quite well. Still I can't find a solution.

解决方案

Concepts that urge you to keep your constructors light weight:

• Inversion of control (Dependency Injection)
• Single responsibility principle (as applied to the constructor rather than a class)
• Lazy initialization
• Testing
• K.I.S.S.
• D.R.Y.

Links to arguments of why:

• How much work should be done in a constructor?
• What (not) to do in a constructor
• Should a C++ constructor do real work?
• http://misko.hevery.com/code-reviewers-guide/flaw-constructor-does-real-work/

If you check the arguments in the constructor that validation code can't be shared if those arguments come in from any other source (setter, constructor, parameter object)

If you fill values into the collection or generate the information strings in the constructor that code can't be shared with other constructors you may need to add later.

In addition to not being able to be shared there is also being delayed until really needed (lazy init). There is also overriding thru inheritance that offers more options with many methods that just do one thing rather then one do everything constructor.

Your constructor only needs to put your class into a usable state. It does NOT have to be fully initialized. But it is perfectly free to use other methods to do the real work. That just doesn't take advantage of the "lazy init" idea. Sometimes you need it, sometimes you don't.

Just keep in mind anything that the constructor does or calls is being shoved down the users / testers throat.

EDIT:

You still haven't accepted an answer and I've had some sleep so I'll take a stab at a design. A good design is flexible so I'm going to assume it's OK that I'm not sure what the information strings are, or whether our object is required to represent a set of numbers by being a collection (and so provides iterators, size(), add(), remove(), etc) or is merely backed by a collection and provides some narrow specialized access to those numbers (such as being immutable).

This little guy is the Parameter Object pattern

/** Throws exception if sign of endValue - startValue != stepSize */
ListDefinition(T startValue, T endValue, T stepSize);

T can be int or long or short or char. Have fun but be consistent.

/** An interface, independent from any one collection implementation */
ListFactory(ListDefinition ld){
    /** Make as many as you like */
    List<T> build();
}

If we don't need to narrow access to the collection, we're done. If we do, wrap it in a facade before exposing it.

/** Provides read access only.  Immutable if List l kept private. */
ImmutableFacade(List l);

Oh wait, requirements change, forgot about 'information strings'. :)

/** Build list of info strings */
InformationStrings(String infoFilePath) {
     List<String> read();
}

Have no idea if this is what you had in mind but if you want the power to count line numbers by twos you now have it. :)

/** Assuming information strings have a 1 to 1 relationship with our numbers */
MapFactory(List l, List infoStrings){
    /** Make as many as you like */
    Map<T, String> build();
}

So, yes I'd use the builder pattern to wire all that together. Or you could try to use one object to do all that. Up to you. But I think you'll find few of these constructors doing much of anything.

EDIT2
I know this answer's already been accepted but I've realized there's room for improvement and I can't resist. The ListDefinition above works by exposing it's contents with getters, ick. There is a "Tell, don't ask" design principle that is being violated here for no good reason.

ListDefinition(T startValue, T endValue, T stepSize) {
    List<T> buildList(List<T> l);
}

This let's us build any kind of list implementation and have it initialized according to the definition. Now we don't need ListFactory. buildList is something I call a shunt. It returns the same reference it accepted after having done something with it. It simply allows you to skip giving the new ArrayList a name. Making a list now looks like this:

ListDefinition<int> ld = new ListDefinition<int>(3, 1, -1);
List<int> l = new ImmutableFacade<int>(  ld.buildList( new ArrayList<int>() )  );

Which works fine. Bit hard to read. So why not add a static factory method:

List<int> l = ImmutableRangeOfNumbers.over(3, 1, -1);

This doesn't accept dependency injections but it's built on classes that do. It's effectively a dependency injection container. This makes it a nice shorthand for popular combinations and configurations of the underlying classes. You don't have to make one for every combination. The point of doing this with many classes is now you can put together whatever combination you need.

Well, that's my 2 cents. I'm gonna find something else to obsess on. Feedback welcome.

这篇关于缺陷：构造器做真正的工作的文章就介绍到这了，希望我们推荐的答案对大家有所帮助，也希望大家多多支持IT屋！