1

I’m having some questions regarding improvement of code structure and maintainability, especially regarding SOLID. These questions are specifically for this project, so I would be happy if this doesn’t end up as a duplicate topic post (I am aware there are a lot of posts regarding this topic).

The main question is, if it is useful or even necessary to use SOLID to properly refactor the code.

A short introduction: I’m working as a coding monkey on a C# .NET project for a windows service. This service has to read text files, which contain data logging information, identify the reading (e.g. voltage) perform some calculations on it and output the data into a text file. This is a single man job I am doing for my employer. It is currently around 3000 LOC As you are well aware project like this start small with a nice small set of requirements. Then suddenly new stuff needs to be implemented within the shortest time possible. As a result the structure of the program is not optimal. I have to admit that I am not very experienced as well (currently studying CS at college).

It started with four classes, one the windows service implementation itself with basic function on the data, and three for the big data structure containing all configuration and identification values.
Now there are two problems, the first is we started with just one type of data logger, now there are multiple and the function which performs the operations on read data needs to be split in parts.
Second and main concern, I still just have four classes. Everything is still in the ConverterService class.

I am aware that according to SOLID and OOD things need be separated in different classes, if they are for different concerns. What I can’t really wrap my head around it, if it really needs to be done here.
For instance I have a function for reading XML and function for writing XML. This could be put in a separate class, but I would just have one instance of it anyway.
Now regarding the data calculation function, this if how the structure currently looks like:

private static void editFile(input the file)
    check the header
    validate that values are really from configured data logger
    check timestamps
    special cases where values registers have certain names do…
    if conditions q
        if data logger x and also a do lots of operations
        else if data logger y and also a do
        else if data logger z and b do
    if condition w
        if data logger x and also a do
        else if data logger y and also a do some other operations
        else if data logger z and b do
    …


As you can imagine pretty unpleasant to look at.

Now what I would definitely do is separate the tasks you can see in different functions (like checking the header etc.) But when it comes to separate the condition operation mix things get a little complicated. I see two ways doing it. First the non OOD/SOLID approach: Keep it in the ConverterService class but create separate functions for the operations. The function editFile will still contains the condition checks and calls these functions.
Second approach, create a class for each data logger with their own operations. It would also be possible to create an interface that each class of data loggers implement to have it more unified. Problem is though, that the operations are very different so probably not the best idea. The function editFile will still contains the condition checks and calls these functions. I don’t see a big improvement because the abstraction doesn’t really simplify anything.

Summarized: 3000 LOC, 37 Functions and one developer

Big question:

  • What should I do?

All questions:

  • Should I use SOLID?
  • Do I need to strongly separate my functions with classes?
  • Have I completely misunderstood OOD? (Have mercy)
  • Is using a new class the right way, even though I’ll just use it to call a function (and have on instance)?
  • How would you refactor this?
  • How to deal with large parameter lists that you need when separating it in several functions?

I really hope this large chunk of questions is not out of place.

closed as too broad by Doc Brown, user22815, user40980, gnat, GlenH7 Aug 5 '15 at 2:06

Please edit the question to limit it to a specific problem with enough detail to identify an adequate answer. Avoid asking multiple distinct questions at once. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • If the x, y, z logs are to be handled separately and differently, they can head off to different handler classes. If there is no common behavior, these handlers don't need to implement the same interface or derive from the same base class. However, based on Interface Segregation Principle, you can still use interfaces to capture some common aspects between them. For example, they may implement the same interface for accepting data. – rwong Jul 31 '15 at 10:34
  • That would be a way and could also solve the long parameter list problem, right? I could create functions or getters and setters for the necessary variables which otherwise would be parameters in the function call. Then when I create an instance, I just need to make sure I have set everything. Though using the constructor should be a lot better though because then I definitely can't miss anything (but have the long list again). – John Jul 31 '15 at 11:25
  • I think your question is too broad in the current form. You ask us to give hints for refactoring a 3000 line program just by a rough scetch. Furthermore you want us to give some general hints about how and when to apply SOLID and/or OOD. To apply SOLID and OOD correctly to your project, there will be one or two dozens of individual problems to be solved, ask about them one at a time. For example, the "parameter list" question is a good fit on its own. – Doc Brown Jul 31 '15 at 12:28
  • Yeah, you're right. What I probably currently need is a general "That would make good class, put that in an interface" rather then a specific "This the specific solution x for problem y". Mainly because I have never done something like that before and I need an idea where to start to have a broad overview on what to do next. – John Jul 31 '15 at 12:37
  • 1
    Back in a beginner's situation, I recalled that I transitioned from "not knowing how to move code around places" to "realizing that you could really move code from anywhere to anywhere and have it do the same thing". From there, it takes lots of experience to learn what would be the natural ways to group things together for the benefit of its human maintainers. Those are summarized by the principles known as SOLID. But that doesn't turn a beginner into an overnight senior programmer; it still takes a long time to learn. The principles only help validate/reaffirm your experience. – rwong Jul 31 '15 at 13:47
1

What should I do?

Whatever you think is best.

You know the requirements. You know the code. You seem to have reasonable views on what is good, what is bad, and what changes might get you.

Should I use SOLID?

Yes. Guidelines exist because they usually will make things better. You shouldn't adhere to it just to be "right" - you should apply the guidelines because you think they'll make your code better.

Do you have unit tests? With code like this, (and being a beginner) you might not. I would suggest writing up some basic ones. They will help point out where your code will be painful to reuse - or where weird coupling points are. That can then help you judge where and how the code should be cleaned up to be better.

  • Unfortunately, no it is currently manually tested. But that is also one of the reasons besides expandability, why I want to refactor it. Shame on me I have to say because I am well aware of that unit tests should be done, but I haven't had the time between the ever changing requirements and studying for college. But in a week I'll have some time to dive into that and hopefully make the working piece of software a beautiful work. – John Aug 1 '15 at 13:52

Not the answer you're looking for? Browse other questions tagged or ask your own question.