缺陷：构造函数完成实际工作

Question

我有一堂课，代表一组数字。构造函数有三个参数：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/

该博客很好地说明了这些问题。我仍然找不到解决方案。

Answer 1

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.