Programming paradigm; wondering if rewriting/refactoring is necessary

Question

for quite some time I've been working on an application. As programming is just a hobby this project is already taking way too long, but that's besides the point. I'm now at a point where every "problem" becomes terribly difficult to solve. And I'm thinking of refactoring the code, however that would result in a "complete" rewrite.

Let me explain the problem, and how I solved it currently. Basically I have data, and I let things to happen over this data (well I described about every program didn't I?). What happens is:

Data -> asks viewer to display -> viewer displays data based on actual data viewer returns user input -> data -> asks "executor" to execute it -> new data

Now this used to work very well, and I was thinking originally "hey I might for example change command prompt by qt, or windows - or even take that external (C#) and simply call this program".

However as the program grew it became more and more tiresome. As the most important thing is that the data is displayed in different manners depending on what the data is and -more importantly- where it is located. So I went back to the tree & added someway to "track" what the parent-line is". Then the general viewer would search for the most specific actual widget. It uses has a list with [location; widget] values, and finds the best matching location.

The problems starts when updating for new "data" - I have to go through all the assets - viewer, saver etc etc. Updating the check-mechanism gave me a lot of errors.. Things like "hey why is it displaying the wrong widget now again?".

Now I can completely swap this around. And instead of the tree datastructure calling to a generic viewer. I would use OO "internal" tree capabilities. The nodes would be childs (& when a new viewer or save-mechanism is needed a new child is formed).

This would remove the difficult checking mechanism, where I check the location in the tree. However it might open up a whole other can of worms. And I'd like some comments on this? Should I keep the viewer completely separate - having difficulty checking for data? Or is the new approach better, yet it combines data & execution into a single node. (So if I wish to change from qt to say cli/C# it becomes almost impossible)

What method should I pursue in the end? Also is there something else I can do? To keep the viewer separate, yet prevent having to do checks to see what widget should be displayed?

EDIT, just to show some "code" and how my program works. Not sure if this is any good as I said already it has become quite a clusterfuck of methodologies.

It is meant to merge several "gamemaker projects" together (as GM:studio strangely lacks that feature). Gamemaker project files are simply sets of xml-files. (Main xml file with only links to other xml files, and an xml file for each resource -object, sprite, sound, room etc-). However there are some 'quirks' which make it not really possible to read with something like boost property trees or qt: 1) order of attributes/child nodes is very important at certain parts of the files. and 2) white space is often ignored however at other points it is very important to preserve it.

That being said there are also a lot of points where the node is exactly the same.. Like how a background can have <width>200</width> and a room too can have that. Yet for the user it is quite important which width he is talking about.

Anyways, so the "general viewer" (AskGUIFn) has the following typedefs to handle this:

    typedef int (AskGUIFn::*MemberFn)(const GMProject::pTree& tOut, const GMProject::pTree& tIn, int) const;
    typedef std::vector<std::pair<boost::regex, MemberFn> > DisplaySubMap_Ty;
    typedef std::map<RESOURCE_TYPES, std::pair<DisplaySubMap_Ty, MemberFn> > DisplayMap_Ty;

Where "GMProject::pTree" is a tree node, RESOURCE_TYPES is an constant to keep track in what kind of resource I am at the moment (sprite, object etc). The "memberFn" will here simply be something that loads a widget. (Though AskGUIFn is not the only general viewer of course, this one is only opened if other "automatic" -overwrite, skip, rename- handlers have failed).

Now to show how these maps are initialized (everything in namespace "MW" is a qt widget):

AskGUIFn::DisplayMap_Ty AskGUIFn::DisplayFunctionMap_INIT() {
    DisplayMap_Ty t;
        DisplaySubMap_Ty tmp;

        tmp.push_back(std::pair<boost::regex, AskGUIFn::MemberFn> (boost::regex("^instances "), &AskGUIFn::ExecuteFn<MW::RoomInstanceDialog>));
        tmp.push_back(std::pair<boost::regex, AskGUIFn::MemberFn> (boost::regex("^code $"), &AskGUIFn::ExecuteFn<MW::RoomStringDialog>));
        tmp.push_back(std::pair<boost::regex, AskGUIFn::MemberFn> (boost::regex("^(isometric|persistent|showcolour|enableViews|clearViewBackground) $"), &AskGUIFn::ExecuteFn<MW::ResourceBoolDialog>));
        //etc etc etc
    t[RT_ROOM] = std::pair<DisplaySubMap_Ty, MemberFn> (tmp, &AskGUIFn::ExecuteFn<MW::RoomStdDialog>);

        tmp.clear();
        //repeat above
    t[RT_SPRITE] = std::pair<DisplaySubMap_Ty, MemberFn>(tmp, &AskGUIFn::ExecuteFn<MW::RoomStdDialog>);
    //for each resource type.

Then when the tree datastructure tells the general viewer it wishes to be displayed the viewer executes the following function:

AskGUIFn::MemberFn AskGUIFn::FindFirstMatch() const {
    auto map_loc(DisplayFunctionMap.find(res_type));
    if (map_loc != DisplayFunctionMap.end()) {
        std::string stack(CallStackSerialize());
        for (auto iter(map_loc->second.first.begin()); iter != map_loc->second.first.end(); ++iter) {
            if (boost::regex_search(stack, iter->first)) {
                return iter->second;
            }
        }
        return map_loc->second.second;
    }

    return BackupScreen;
}

And this is where the problems began to be frank. The CallStackSerialize() function depends on a call-stack.. However that call_stack is stored inside a "handler". I stored it there because everything starts FROM a handler. I'm not really sure where I ought to store this "call_stack". Introduce another object that keeps track of what's going on? I tried going the route where I store the parent with the node itself. (Preventing the need for a call-stack). However that didn't go as well as I wished: each node simply has a vector containing its child nodes. So using pointers is out of the question to point to the parent note... (PS: maybe I should reform this in another question..)

Answer 1

Refactoring/rewriting this complicated location checking mechanism out of the viewer into a dedicated class makes sense, so you can improve your solution without affecting the rest of your program. Lets call this NodeToWidgetMap.

Architecture
Seems your heading towards a Model-View-Controller architecture which is a good thing IMO. Your tree structure and its nodes are the models, where as the viewer and the "widgets" are views, and the logic selecting widgets depending on the node would be part of a controller.

The main question remains when and how you choose the widget w_N for a given node N and how to store this choice.

NodeToWidgetMap: When to choose
If you can assume that w_N does not change during its lifetime even though nodes are moved, you could choose it right when creating the node . Otherwise you'll need to know the location (or path through the XML) and, in consequence, find the parent of a node when requesting it.

Finding Parent Nodes
My solution would be to store pointers to instead of the node instances themselves, perhaps using boost::shared_ptr. This has drawbacks, for example copying nodes forces you to implement your own copy-constructors that uses recursion to create a deep-copy of your sub-tree. (Moving however will not affect the child nodes.)

Alternatives exist, such as keeping child nodes uptodate whenever touching the parent node respective the grandfathers vector. Or you can define a Node::findParentOf(node) function knowing that certain nodes can only (or frequently) be found as child of certain nodes. This is brute but will work reasonably well for small trees, just does not scale very well.

NodeToWidgetMap: How to choose
Try writing down the rules how to choose w_N on piece of paper, perhaps just partially. Then try to translate these rules into C++. This might slightly longer in terms of code but will be easier to understand and maintain.

Your current approach is to use regular expressions for matching the XML path (stack).

My idea would be to create a lookup graph whose edges are labelled by the XML element names and whose nodes indicate which widget shall be used. This way your XML path (stack) describes a route through the graph. Then the question becomes whether to explicitly model a graph or whether a group of function calls could be used to mirror this graph.

NodeToWidgetMap: Where to store choice
Associating a unique, numeric id to each node, record the widget choice using a map from node id to widget inside the NodeToWidgetMap.

Rewriting vs Refactoring
If you rewrite you might get good leverage tieing to an existing framework such as Qt in order to focus on your program instead of rewriting the wheels. It can be easier to port a well-written program from on framework to another than to abstract around the pecularities of each platform. Qt is a nice framework for gaining experience and good understanding of the MVC-architectures.

A complete rewrite gives you a chance to rethink everything but implies the risk that you start from scratch and will be without a new version for a good amount of time. And who knows whether you will have enough time to finish? If you choose to refactor the existing structures you will improve it step by step, having a useable version after each step. But there is small risk to remain trapped in old ways of thinking, where as rewriting nearly forces you to rethink everything. So both approaches have their merits, if you enjoy programming I would rewrite. More programming, more joy.

Answer 2

Welcome to the world of programming!
What you describe is the typical life cycle of an application, starts as a small simple app, then it gets more and more features until it is no longer maintainable. You can't imagine how many projects I've seen in this last collapsing phase!
Do you need to refactor? Of course you do! All the time! Do you need to rewrite everything? Not so sure.
In fact the good solution is to work by cycles: you design what you need to code, you code it, you need more functionality, you design this new functionality, you refactor the code so you can integrate the new code, etc. If you don't do it like this then you will arrive to the point where its less expensive to rewrite then to refactor. Get this book: Refactoring - Martin Fowler. If you like it then get this one: Refactoring to Patterns.

Answer 3

As Pedro NF said, Martin Fowler "Refactoring" is the nice place to get familiar with it.

Answer 4

I recommend buying a copy of Robert Martins "Agile Principles, Patterns and Practices in C#" He goes over some very practical case studies that show how to overcome maintenance problems like this.