Is this pattern bad? [duplicate]

I notice that when I code I often use a pattern that calls a class method and that method will call a number of private functions in the same class to do the work. The private functions do one thing only. The code looks like this:

public void callThisMethod(MyObject myObject) {
    methodA(myObject);
    methodB(myObject);  // was mehtodB before
}

private void methodA(MyObject myObject) {
    //do something
}

 private void methodB(MyObject myObject) {
    //do something
}

Sometimes there are 5 or more private methods, in the past I have moved some of the private methods into another class, but there are occasions when doing so would be creating needless complexity.

The main reason I don’t like this pattern is that is not very testable, I would like to be able to test all of the methods individually, sometimes I will make all of the methods public so can I can write tests for them.

Is there a better design I can use?

3

It’s a good pattern. This is what Uncle Bob refers to when he talks about extracting til you drop.

It shouldn’t affect your tests though. You should only be testing the public interface. Yes, you can reflect your way in and test the private methods, but you’re still going to have to test them again when you test your public methods, so it doesn’t solve the perceived problem.

The “problem” comes when you’re testing a private method multiple times because you have multiple public methods using them.

Sometimes, in fact more frequently than you might think, this doesn’t matter. Particularly if you’re testing by behaviour, rather than trying to test each line of code. All you want to know is “Does this public method do what it’s supposed to do?” You don’t care about the implementation details.

It does matter, however, when you’re testing the same piece of complex logic repeatedly, because it adds multiple tests to each behaviour. At that point, simply extract it into a service and mock it out.

9

What I would keep an eye out for though is the amount of times you’re passing around your initial object parameter. If all of your private methods are operating on the main parameter, it may be a bit of a code smell. E.g. if you have:

class Foo {
    public void callThisMethod(Bar myBarObject) {
        methodA(myBarObject);
        methodB(myBarObject);
        methodC(myBarObject);
        methodD(myBarObject);
        methodE(myBarObject);

        …

        methodZ(myBarObject);

    }
}

class Bar {
    …
}

This points to low cohesion between the methods of Foo with that class — The methods seem to have more in common with Bar than with Foo. If that’s the case, consider moving related methods into the class that they’re operating on (see the Law of Demeter).

If that’s not the case though, there’s nothing wrong with having methods that do just one thing, or with testing your application via it’s public methods rather than the private internals (as you can still ‘exercise’ all of the use cases of these methods, but it leaves you freer to refactor these methods later on if you so choose).

3

I’m wondering if you’re trying to implement the Command Pattern via private methods, and you might find switching to a command pattern easier for what you are doing.

What you have now is this:

public void callThisMethod(MyObject myObject) {
    methodA(myObject);
    mehtodB(myObject)
}

private void methodA(MyObject myObject) {
    //do something
}

private void methodB(MyObject myObject) {
    //do something
}

You could implement the same thing using commands and a dispatcher.

public interface Command {
    void Execute(Context c);
}

public class Context {
    // this holds shared data
}

public class Dispatcher
{
    private List<Command> _commands = new List<Command>();

    public void Add(Command cmd) 
    {
       _commands.Add(cmd);
    }

    public void Run(Context c)
    {
       foreach(Command cmd in _commands) { cmd.Execute(c); }
    }
}

You would then implement your private methods now as commands.

public class CommandA : Command { ... }
public class CommandB : Command { ... }

Any special state information is defined in the Context class which will be passed to each command. To run the code you would add the commands to the dispatcher and run it.

Dispatcher d = new Dispatcher();
d.Add(new CommandA());
d.Add(new CommandB());
d.Run(new Context());

The advantage of this approach is that it decouples the commands from each other, and you use the Context class to make everything work together. You can also add logic to what commands are created, and what parameters are used to instantiate them.

Some other tips, you can create singletons of commonly used commands to reduce overhead in creating new ones all the time.

This approach is highly testable as each command can be isolated.

The downside is that a large collection of commands can become difficult to maintain. It’s better to create fat commands that can do a lot of things, than simple small commands in large number.

1

I believe that the pattern in question is known as composed method and it is mentioned in Kent Becks Smalltalk Best practice patterns [1]. Further, he mentions that each method does one identifiable thing and that the methods called within a method should all be at the same level of abstraction.

[1] http://www.amazon.com/Smalltalk-Best-Practice-Patterns-Kent/dp/013476904X

For me this seems like a bad practice for two reasons.

It is unclear how those methods depend on each other. If there is an order to strictly adhere to then this order should be reflected in the code. Otherwise a hidden temporal coupling (see item G31 in Clean Code) is introduced. Making the coupling obvious can be achieved by returning an object from every method and passing that on to the next method.

If those methods don’t really depend on each other an interface should be introduced with a single method. The different implementations should be kept in a collection in a field of the owning class of callThisMethod and that method should only iterate through that collection and invoke the collection elements method. This way you are adhering more to the open closed principle, because adding another functionality is just the matter of configuring another interface implementation to be kept in that list and not changing the owning class of callThisMethod

Trang chủ Giới thiệu Sinh nhật bé trai Sinh nhật bé gái Tổ chức sự kiện Biểu diễn giải trí Dịch vụ khác Trang trí tiệc cưới Tổ chức khai trương Tư vấn dịch vụ Thư viện ảnh Tin tức - sự kiện Liên hệ Chú hề sinh nhật Trang trí YEAR END PARTY công ty Trang trí tất niên cuối năm Trang trí tất niên xu hướng mới nhất Trang trí sinh nhật bé trai Hải Đăng Trang trí sinh nhật bé Khánh Vân Trang trí sinh nhật Bích Ngân Trang trí sinh nhật bé Thanh Trang Thuê ông già Noel phát quà Biểu diễn xiếc khỉ Xiếc quay đĩa Dịch vụ tổ chức sự kiện 5 sao Thông tin về chúng tôi Dịch vụ sinh nhật bé trai Dịch vụ sinh nhật bé gái Sự kiện trọn gói Các tiết mục giải trí Dịch vụ bổ trợ Tiệc cưới sang trọng Dịch vụ khai trương Tư vấn tổ chức sự kiện Hình ảnh sự kiện Cập nhật tin tức Liên hệ ngay Thuê chú hề chuyên nghiệp Tiệc tất niên cho công ty Trang trí tiệc cuối năm Tiệc tất niên độc đáo Sinh nhật bé Hải Đăng Sinh nhật đáng yêu bé Khánh Vân Sinh nhật sang trọng Bích Ngân Tiệc sinh nhật bé Thanh Trang Dịch vụ ông già Noel Xiếc thú vui nhộn Biểu diễn xiếc quay đĩa Dịch vụ tổ chức tiệc uy tín Khám phá dịch vụ của chúng tôi Tiệc sinh nhật cho bé trai Trang trí tiệc cho bé gái Gói sự kiện chuyên nghiệp Chương trình giải trí hấp dẫn Dịch vụ hỗ trợ sự kiện Trang trí tiệc cưới đẹp Khởi đầu thành công với khai trương Chuyên gia tư vấn sự kiện Xem ảnh các sự kiện đẹp Tin mới về sự kiện Kết nối với đội ngũ chuyên gia Chú hề vui nhộn cho tiệc sinh nhật Ý tưởng tiệc cuối năm Tất niên độc đáo Trang trí tiệc hiện đại Tổ chức sinh nhật cho Hải Đăng Sinh nhật độc quyền Khánh Vân Phong cách tiệc Bích Ngân Trang trí tiệc bé Thanh Trang Thuê dịch vụ ông già Noel chuyên nghiệp Xem xiếc khỉ đặc sắc Xiếc quay đĩa thú vị
Trang chủ Giới thiệu Sinh nhật bé trai Sinh nhật bé gái Tổ chức sự kiện Biểu diễn giải trí Dịch vụ khác Trang trí tiệc cưới Tổ chức khai trương Tư vấn dịch vụ Thư viện ảnh Tin tức - sự kiện Liên hệ Chú hề sinh nhật Trang trí YEAR END PARTY công ty Trang trí tất niên cuối năm Trang trí tất niên xu hướng mới nhất Trang trí sinh nhật bé trai Hải Đăng Trang trí sinh nhật bé Khánh Vân Trang trí sinh nhật Bích Ngân Trang trí sinh nhật bé Thanh Trang Thuê ông già Noel phát quà Biểu diễn xiếc khỉ Xiếc quay đĩa
Thiết kế website Thiết kế website Thiết kế website Cách kháng tài khoản quảng cáo Mua bán Fanpage Facebook Dịch vụ SEO Tổ chức sinh nhật