Is it possible to make long code representing a computation easier to read?

Question

Long methods are generally considered bad, however in my code I have some hard-to-understand long methods (more than 50 lines). I have trouble making those methods easier to read because a single statement inside is already more than 50 lines long, and that hard-to-read single statement is to build a database query using an ORM to do some specific job where the job done is clearly indicated on the method name. The reason that the statement is so long because it joins on multiple columns, applies multiple wheres and selects multiple distinct columns to make a required documented output format.

Is such hard-to-read code considered bad code? Similarly, if I write code for a complicated algorithm resulting in hard-to-read code wrapped in a clearly-named method, is that code bad code?

Answer 1

You wrote

Is such hard-to-read code considered bad code

so you definitely agree it is hard-to-read code, and if it is hard to read, it is hard to maintain and evolve - so I guess you consider the code as beeing "bad" by your own measures. However, sometimes it is not obvious how to improve something like a 50 line SQL statement. The easy "extract method" refactorings don't work, and you probably do not have a clue where to start in making the code more readable. For these cases, you can still try one or all of the following

• show the code someone else who is more experienced than you in cleaning up code. If you do not have someone in your organization you can ask, try codereview.stackexchange

• try to google for the specific problem. For your example, things like "clean up long sql statement" might be a good start. You will be astonished how many articles you find about that topic, and even if you cannot find a braindead recipe for your case, you might get some fresh ideas

• instead of asking for a justification for the things you cannot do, focus on the things you can do to clean up the code at least a little bit, like adding proper line breaks, proper indentation, adding some explaining comments, giving some variables a better name. It is not unlikely, during this process, forcing yourself to reread the details of the code, you find a way to refactor the code into smaller units

• practice, practice, practice. "Clean coding" is not something you learn in one day, it get easier with more experience. Maybe you do not find a solution for your problem today, but when you come back to the code in a few months, it will look differently to you.

Answer 2

Hard to read isn't bad - unnecessarily hard to read is bad.

Some things just are difficult. In that case, you need to completely understand the problem, write the code, and comment it as good as you can so the next developer has a chance.

But some things are only difficult because you made them difficult. If the problem can be simplified and made easier, simplify it. If it is hard to understand but can be reasonably split into subproblems, then split it into subproblems to simplify it.

Answer 3

ORM to create a report? Seriously? Learn some SQL, man. Procedural languages are terrible at this sort of thing.

SQL is a language much better designed to handle complicated joins and selects. And even if you can't get the SQL to look beautiful, there are all kinds of visualization tools available where you can drag and drop database objects on a diagram and the SQL will get generated for you. Not to mention you will be able to tune and optimize the query, view its query plan, get the platform to suggest additional indexing options, etc. It's just way more flexible.

I'm sure some folks on here will disagree with me, but ORM is not a good fit for complicated reporting purposes. If at all possible I would consider moving away from that and moving toward Structured Query Language.

Answer 4

In general hard to read code is a bad idea anywhere - even if you are the only maintainer - I have had several occurrences of returning to some code years or even weeks later and finding it hard to get my head around.

If you need to do a lot in a single query try splitting it across lines with embedded comments:

query(join(map(condition1, condition2), blah, blah, something(bah, blah, blah))) 

Becomes:

// Why are we doing such an awful single query rather than a sequence of selects?
query( // Description of query
  join( // Why are you doing a join here
    map( // Why a map
      condition1, // What is this
      condition2  // And this
   ), // End Map
   blah, // What, Why?
   blah, // What, Why?
   something( // What does this do?
      bah, // What, Why?
      blah, // What, Why?
      blah // What, Why?
      ) // End Something
   ) // End Join
) // End Query

Answer 5

In addition to @DocBrown's excellent answer, I think it's worth recognising that nobody writes "good" code all the time. Coding is making trade-offs, and often it's better to accept that you've written something a little bit less clean than you'd like and come back to it later. Refactoring is the process of improving code over time - and in my experience, that's what makes a good code base, not "get it right first time".

And you assess code at the level of the application, not the level of individual methods/lines of code. So if you have a complex method, but it's clearly named, I don't think you have "bad" code as long as the method is cohesive.

Naming is the biggest weapon you have in making code intelligible - give your method a name which allows people reading your code to skip the body if they need to. Name your variables etc. in a way that means readers can see what they represent without having to read their assignment statements.

The next thing is to make sure your method really does only one thing - side effects make code hard to understand. So if your method returns data for an output format, it shouldn't also update the status of your database to "printed", or whatever.

Layering/componentization is the next thing you might do - if you have a bunch of complex methods which generate ORM results, bundle them together so readers of your code can assume that they all behave in the same way, have no side effects etc.