Question

I'm an electrical engineer and I don't know what the hell I'm doing. Please save the future maintainers of my code.

Recently I've been working on a number of smaller programs (in C#) whose functionality is logically "procedural". For example, one of them is a program which collects information from different databases, uses that information to generate a sort of summary page, prints it, and then exits.

The logic required for all of that is around 2000 lines. I certainly don't want to stuff all of that in a single main() and then "clean it up" with #regions as some previous developers have been doing (shudder).

Here are some things I have already tried without much satisfaction:

Make static utilities for each coarse bit of functionality, e.g. a DatabaseInfoGetter, SummaryPageGenerator, and a PrintUtility. Making the main function look like:

int main()
{
    var thisThing = DatabaseInfoGetter.GetThis();
    var thatThing = DatabaseInfoGetter.GetThat();
    var summary = SummaryPageGenerator.GeneratePage(thisThing, thatThing);

    PrintUtility.Print(summary);
}

For one program I even went with interfaces

int main()
{
    /* pardon the psuedocode */

    List<Function> toDoList = new List<Function>();
    toDoList.Add(new DatabaseInfoGetter(serverUrl));
    toDoList.Add(new SummaryPageGenerator());
    toDoList.Add(new PrintUtility());

    foreach (Function f in toDoList)
        f.Do();
}

None of this feels right. As the code gets longer, both of these approaches start to get ugly.

What's a good way to structure stuff like this?

Was it helpful?

Solution

Since you are not a professional programmer, I would recommend sticking to simplicity. It will be a LOT easier for programmer to take your modularized, procedural code and make it OO later, than it will be for them to fix a badly written OO program. If you are not experienced, it is possible to create OO programs that can turn into an unholy mess which will neither help you or whoever comes after you.

I think your first instinct, the "this thing-that thing" code in the first example is the right track. It is clear and obvious what you want to do. Don't worry too much about code efficiency, clarity is much more important.

If a code segment is too long, break it into bite-sized chunks, each having its own function. If it is too short, consider using less modules and putting more in line.

---- Postscript: OO Design Traps

Working successfully with OO programming can be tricky. There are even some people who consider the whole model flawed. There is a very good book I used when first learning OO programming called Thinking in Java (now in its 4th edition). The same author has a corresponding book for C++. There is actually another question on Programmers dealing with common pitfalls in object-oriented programming.

Some pitfalls are sophisticated, but there are plenty of ways to create problems in very basic ways. For example, a couple of years ago there was an intern at my company who wrote the first version of some software I inherited and he made interfaces for everything that might someday have multiple implementations. Of course, in 98% of the cases there was only a single implementation ever, so the code was loaded with unused interfaces which made debugging very annoying because you can't step back through an interface call, so you end up having to do a text search for the implementation (although now I use IntelliJ was has a "Show All Implementations" feature, but back in the day I didn't have that). The rule here is the same as for procedural programming: always hardcode one thing. Only when you have two or more things, create an abstraction.

A similar kind of design blunder can be found in the Java Swing API. They use a publish-subscribe model for the Swing menu system. This makes creating and debugging menus in Swing a complete nightmare. The irony is that it is completely pointless. There is virtually never a situation in which multiple functions would need to "subscribe" to a menu click. Also, publish-subscribe was a complete misfire there because a menu system is normally always in use. It's not like functions are subscribing and then unsubscribing randomly. The fact that "professional" developers at Sun made a blunder like this just shows how easy it is even for pros to make monumental screw ups in OO design.

I am a very expert developer with decades of experience in OO programming, but even I would be the first to admit there is tons I don't know and even now I am very cautious about using a lot of OO. I used to listen to long lectures from a co-worker who was an OO zealot about how to do particular designs. He really knew what he was he was doing, but in all honesty I had a hard time understanding his programs because they had such sophisticated design models.

OTHER TIPS

The first approach is fine. In procedural languages, like C, the standard approach is to divide functionality into namespaces - using static classes like DatabaseInfoGetter is basically the same thing. Obviously this approach leads to simpler, more modular and more readable/maintainable code than shoving everything into one class, even if everything is broken down into methods.

Speaking of which, try to limit methods to performing as few actions as possible. Some programmers prefer a little less granularity, but huge methods are always considered harmful.

If you're still struggling with complexity, you most likely need to break down your program even further. Create more levels in the hierarchy - maybe DatabaseInfoGetter needs to reference some other classes like ProductTableEntry or something. You're writing procedural code but remember, you ARE using C#, and OOP gives you a bunch of tools to reduce complexity, e.g.:

int main() 
{
    var thisthat = Database.GetThisThat();
}

public class ThisThatClass
{
    public String This;
    public String That;
}

public static class Database 
{
    public ThisThatClass GetThisThat()
    {
        return new ThisThatClass 
        {
            This = GetThis(),
            That = GetThat()
        };
    }

    public static String GetThis() 
    { 
        //stuff
    }
    public static String GetThat() 
    { 
        //stuff
    }

}

Also, be careful with static classes. Database classes are good candidates.. as long as you usually have one database.. you get the idea.

Ultimately if you think in mathematical functions and C# doesn't suit your style, try something like Scala, Haskell, etc. They're cool too.

I'm responding 4 years late but when you're trying to do things procedural in an OO language then you're working on an antipattern. That's not something you should do! I found a blog that addresses this antipattern and shows a solution for it, but it requires a lot of refactoring and a change in mindset. See Procedural code in Object-Oriented code for more details.
As you mentioned "this" and "that", you are basically suggesting that you have two different classes. A This class (bad name!) and a That class. And you could e.g. translate this:

var summary = SummaryPageGenerator.GeneratePage(thisThing, thatThing);

into this:

var summary = SummaryPageGenerator.GeneratePage(new This(DatabaseInfoGetter), new That(DatabaseInfoGetter));

Now, it would be interesting to see those 2,000+ lines of procedural code and determine how various parts could be grouped together in separate classes and move various procedures down into those classes.
Each class should be simple and focused on just doing one thing. And it seems to me that some parts of the code is already dealing with superclasses, that are actually way too big to be useful. The DatabaseInfoGetter, for example, sounds way to confusing. What is it getting anyways? It sounds too generic. Yet SummaryPageGenerator is awfully specific! And PrintUtility is being generic again.
So, to start, you should avoid the generic names for your classes and start using more specific names instead. The PrintUtility class, for example, Would be more a Print class with a Header class, a Footer class, a TextBlock class, an Image class and possibly some way to indicate how they would flow in the print itself.All this could be in the PrintUtility namespace, though.
For the database, similar story. Even if you use the Entity Framework for database access, you should have it limited to the things you use, and have it divided into logical groups.
But I wonder if you're still dealing with this problem after 4 years. If so, then it's a mindset that needs to be changed away from the "procedural concept" and into the "object oriented concept". Which is challenging if you don't have the experience...

I'm my opinion, you should change your code so that it is Simple, Consistent and Readable.

I wrote some blog posts on this topic, and trust me they are simple to understand: What is good code

Simple: Just like a circuit board contains different pieces, each one with a responsibility. Code should be divided into smaller, simpler parts.

Consistent: Use patters, a standard for naming elements and things. You like tools that work the same way all the time and you want to know where to find them. It is the same with code.

Readable: Don't use acronyms unless they are widely used and known within the domain the software is targeting (mattnz), prefer pronounceable names that are clear and easy to understand.

Licensed under: CC-BY-SA with attribution
scroll top