Take the 2-minute tour ×

Someone recently took a look of my code and commented that it was too procedural. To be clear, it was not much of the code they saw - just a section which clearly outlines the logical steps taken in the application.

if(downloadFeeds(ftpServer, ftpUsername, ftpPassword, getFtpPathToLocalPathMap())) {

These various methods then create a whole bunch of different objects, and call various methods on these objects as required.

My question is - is a section of code which clearly drives your application, such as this, indicative of procedural programming, and if so what would be a more OO way to achieve the same outcome?

All comments are much appreciated!

share|improve this question
We sometimes seem to forget the the excuse for OOP was better maintainability. So ask yourself if you code is clear to others who never saw it before and will they have hard time making changes. Can this code be used elsewhere in your environment with slight mods? (if you are going to cut-and-paste this module elswhere, maybe OO could help here) ... Without knowing about your app and what all the functions mean its hard to tell though if concern is warranted. –  nhed May 13 '11 at 22:53
It all comes down to re-usability. I imagine this is all in some main() method and all of these methods are statics. Now, if you had to change something, maybe you had an additional Z field, and now you needed to add process Z and resume Z and so forth.. with your code, it would be difficult to do so without major refactoring. If you had gone the OO route, it would be as simple as initiating another class to handle that. That's the power of OOP. –  Mohamed Nuur May 13 '11 at 22:59
I have a feeling there are a lot of globals. This code is 100% untestable. –  James Jones May 13 '11 at 23:01

9 Answers 9

up vote 7 down vote accepted

Well, the class in which this piece of code resides has clearly too much responsibilities. I wouldn't go as far as hide all that stuff in a facade but instead of having all stuff related to some ftp engine, datasources and other entities located in a single god object (anti pattern), there should be a business process which HAS all these kinds of entities.

So the code could look more like that:

    // ...

Datasource, engine and all the other components may be Injected into your business process, so a) testing gets way easier, b) swapping implementations is simplified and c) code reuse is possible.

Please note that a procedural looking piece of code isn't always bad:

class Person {
    public void getMilk() 
        Glass glass = cupboard.getGlass(); 
        Milk milk = fridge.getMilk(); 
        use(glass, milk);
    // More person-ish stuff

While that code clearly looks procedural, it may be fine. It makes totally clear what happens here and does not need any documentation (clean code from martin encourages code like that). Just keep the single responsibility principle and all the other basic OOP rules in mind.

share|improve this answer
it is better if you would edit your "goto" method call because it is a reserved word in java and it makes confusions. Just because your "goto" statement is caught by syntax-formatter colored with blue. –  Erhan Bagdemir May 13 '11 at 23:14
Thats only some senseless piece of pseudo code, but yeah, your right. –  atamanroman May 13 '11 at 23:15
i know what you mean, but it just irritates. thanks for edit. here your +1 –  Erhan Bagdemir May 13 '11 at 23:19
Thanks for the hint, I did not get that ;) –  atamanroman May 13 '11 at 23:19
@mohamed thats a fair point, but I don't think the glass should be responsible to get filled by the milk. Maybe there should be no getMilk method at all, just a generic drink(Drink d) one? Design is fun! –  atamanroman May 13 '11 at 23:35

Wow ! It doesn't seem like OO-Style. How about like this:

    ConnectionData cData = new ConnectionData(ftpServer, ftpUsername, ftpPassword, getFtpPathToLocalPathMap());

    if(downloadFeeds(cData)) {
      MyJobFacade fc = new MyJobFacade(); 


public class MyJobFacade {

    public void doYourJob() {
          /* all your do operations maybe on different objects */

by the way Facade Pattern http://en.wikipedia.org/wiki/Facade_pattern

share|improve this answer
The facade pattern may help in cases where you want to hide some ugly apis or calls to code widely spread in your problem domain, but if the sample code is a real business process I wouldn't try to hide it. Breaking up all the responsibilities may be enough to create clean, nice looking code in that case. –  atamanroman May 13 '11 at 23:11

I agree that it looks too procedural. One obvious way to make it look less procedural is to have all of these methods be a part of a class. Erhan's post should give you a better idea of how to break it up :-)

share|improve this answer

Yes. Your methods don't return any values so from the snippet it appears they're operating on global variables. It looks like textbook procedural programming.

In a more OO approach I'd expect to see something like this:

  MyDatasources ds = loadDataSources();
  Engine eng = initEngine(ds);
  DataObj data = loadLiveData(eng);
  Id[] xIds = processX(data.getX());
  Id[] newXIds = xIds.clone();
  Id[] yIds = processY(data.getY());
  Id[] newYIds = yIds.clone();


share|improve this answer
I'm not sure the snippet here is more "object-oriented", but it's clearly superior to the original. And also superior to most of the other answers, which try to "solve" the problem by hiding the global variable mess behind more classes. –  bkail May 14 '11 at 0:01

Here's how I've learned to differentiate:

A sign that your code is getting "procedural" on you is when a class has more than one responsibility (See this link on the Single Responsibility Principle). It looks like all of these methods are being called by one object, which means that one class is managing a bunch of responsibilities.

A better way would be to assign these responsibilities to the real-world object that can best handle them. Once these responsibilities have been properly delegated, implementing a software pattern that can efficiently drive these functions (such as the Facade pattern).

share|improve this answer

This is indeed procedural programming. If you're trying to make it more OO, I'd try a combination of these:

  • Try to have classes that represent the data you're holding. For example, have a FeedReader class to handle feeds, a DataLoader class to load the data, etc.
  • Try to separate the methods into classes with cohesive functionality. For example, group resume(), pause() into the previous FeedReader class.
  • Group the process() methods into a ProcessManager class.

Basically, just try to think of your program as having a bunch of employees working for you, and you need to assign them responsibilities. (PM do this for me, then DM do this with the result of that). If you give all the responsibility to one employee, he or she is going to be burned out.

share|improve this answer


It is not possible to say if this code fragment is "too procedural".

  • Those calls could all be to instance methods on the current object, operating on either separate objects, or on instance variables of the current instance. These would make the code OO, at least to some degree.

  • If those methods are static, then the code is indeed procedural. Whether this is a bad thing depends on whether the methods are accessing and updating state stored in static fields. (Judging from the names, they probably would need to do that.)


It is hard to say without looking at the rest of the code, but there appear to be some implied objects in the picture; e.g.

  • data sources and (maybe) a data source manager or registry
  • an engine of some kind
  • something to hold the live data, the X's and the Y's
  • and so on.

Some of these probably need to be classes (if they are not already classes) ... but which ones depends on what they are doing, how complicated they are and so on. State that doesn't make sense as classes (for whatever reason) can be made "more OO" by turning static variables into instance variables.

Other Answers suggest specific refactorings, getting rid of all globals, use of dependency injection and so on. My take is that it there is insufficient information to judge whether these suggestions are going to help.

But is it worth it?

Just making an application "more OO" is not a useful or worthwhile goal. Your aim should be to make the code more readable, maintainable, testable, reusable, etc. Whether using OO will improve matters depends on the current state of your code, and of the quality of your new design and the refactoring work. Simply adopting OO practices will not correct a poor design, or turn "bad" code into "good" code.

share|improve this answer

I’m going to take a different approach to critiquing this than whether or not it’s ‘too procedural’ or not. I hope you find it somewhat useful.

First off I don't see any function parameters or return values. This mean you’re probably using all kinds of global data, which should be avoided for numerous good reasons that you can read about here if you like: Are global variables bad?

Second, I don't see any error checking logic. Let’s say resumeY fails with an exception, maybe the problem is in resumeY but it could also be higher up in pauseY or as far up as loadDataSources and the problem only manifests as an exception later on.

I don’t know if this is production code or not but it’s a good candidate for re-factoring in several stages. In the first stage you can go through and make each function return a Boolean success or not and in the body of each function check for known error cases. After you have some error checking in there start getting rid of your global data by passing in function args and returning result data; you can make your functions return null values in case of failure or convert over to exception handling, I suggest exceptions. After that think about making the individual pieces testable in isolation; e.g. so you can test downloadFeeds separate from the data processing function and vice versa.

If you go through and do some re-factoring you’ll start seeing the obvious places where you can modularize your code and improve it. IMO you should worry less about if you’re OOP-enough and more about whether or not you can 1. Debug it effectively, 2. Test it effectively and 3. Understand it after leaving it alone and coming back 6 months later to maintain it.

This reply ended up quite long, I hope you found parts of it useful. :-)

share|improve this answer

My best advice would be Clean Code: A Handbook Of Agile Software Craftsmanship

This is an excellent book of code style best practices. A bit long, but worth the time.

share|improve this answer

Your Answer


