220

Sometimes, the best way to know, is to come back to code you wrote six months ago and try and understand what it was written to do. If you understand it quickly - it's readable.


177

There is one big reason why regular expressions were designed as terse as they are: they were designed to be used as commands to a code editor, not as a language to code in. More precisely, ed was one of the first programs to use regular expressions, and from there regular expressions started their conquest for world domination. For instance, the ed command ...


120

As others have already mentioned: don't create a function with a name that is similar to that of a builtin, standard-library or generally widely used function but change its behavior. It is possible to get used to a naming convention even if it doesn't make much sense to you at first sight but it will be impossible to reason about the functioning of your ...


118

You cross the line when You have measured that your code is too slow for its intended use. You have tried alternative improvements that don't require mucking up the code. Here's a real-world example: an experimental system I am running was producing data too slowly, taking over 9 hours per run and using only 40% of CPU. Rather than mess up the code too ...


115

If you make a function like that where minimize(4, 10) returns 10, then I'd say that is inadvisable because your fellow programmers may strangle you. (Okay, maybe they will not literally strangle you to death, but seriously... Don't do that.)


111

If you felt compelled to expand a one liner like a = F(G1(H1(b1), H2(b2)), G2(c1)); I wouldn't blame you. That's not only hard to read, it's hard to debug. Why? It's dense Some debuggers will only highlight the whole thing at once It's free of descriptive names If you expand it with intermediate results you get var result_h1 = H1(b1); var result_h2 ...


93

It is: maintainable if you can maintain it. easily maintainable if someone else can maintain it without asking you for help readable if someone else, on reading it, correctly understands the design, layout and intent The real test for 1. is (as Alex in Paris and quant_dev say) that you can pick it back up after a few months doing something else. The test ...


93

I could overload the constructor so that order [of the parameters] doesn't matter... But is that a good idea? No. Having different constructor overloads will have the opposite effect of what you are intending. The programmer coming after you expects different overloads to have different behavior, and will ask: "What sort of different behavior is being ...


88

What is easier to understand The latter approach. It's not only easier to understand but it is easier to write, test, refactor and extend as well. Each required condition can be safely decoupled and handled in it's own way. it's problematic because you have to read all the methods to understand the code It's not problematic if the methods are named ...


87

... when arguing with one of my collegues, who is going to the point of declaring constants like: private const char SemiColon = ';'; private const char Space = ' '; private const int NumberTen = 10; The argument you need to be making with your colleague isn't about naming a literal space as Space but his poor choice of name for his constants. Let's say ...


84

The LoC in a method is a completely pointless measure. The important things are separation of concerns and code duplication. A method should only do one thing, and that one thing should be expressed in its name. Other things should be left to other methods. The problems arising from code duplication cannot be overestimated (in other words, they are always ...


80

No, long methods are not always bad. In the book Code Complete, it is measured that long methods are sometimes faster and easier to write, and don't lead to maintenance problems. In fact, what is really important is to stay DRY and respect separation of concerns. Sometime, the computation is just long to write, but really won't cause issue in the future. ...


77

Yes, splitting long functions is normal. This is a way of doing things that's encouraged by Robert C. Martin in his book Clean Code. Particularly, you should be choosing very descriptive names for your functions, as a form of self-documenting code.


63

When reviewing code, I apply the following rules: Always use const for function parameters passed by reference where the function does not modify (or free) the data pointed to. int find(const int *data, size_t size, int value); Always use const for constants that might otherwise be defined using a #define or an enum. The compiler can locate the data in ...


62

I would regard this as an appropriate place to use command/query separation. For example: // query var validItems = items.Where(i => i.Field != null && i.State != ItemStates.Deleted); // command foreach (var item in validItems) { // do stuff } This also allows you to give a good self-documenting name to the query result. It also helps you ...


60

All names should be meaningful. If _ was a well known standard at your company or in the wider community, then it would be meaningful as a "name that does not matter". If it's not, I would say it's bad practice. Use a descriptive name for what you refer to, especially since the name might matter in the future.


60

Nested loops are fine as long as they describe the correct algorithm. Nested loops have performance considerations (see @Travis-Pesetto's answer), but sometimes it's exactly the correct algorithm, e.g. when you need to access every value in a matrix. Labeling loops in Java allows to prematurely break out of several nested loops when other ways to do this ...


58

IMO, the previous answers focus on edge cases and code which should be rewritten anyway. Here are some cases I see nearly everywhere in which if/else should have been replaced by if or another language construct. Note: I make the answer community wiki. Feel free to modify it and add other examples of cases where if/else usually leads to bad code. Beginners,...


57

It's a false dichotomy. You can make code fast and easy to maintain. The way you do it is write it clean, especially with as simple a data structure as possible. Then you find out where the time drains are (by running it, after you've written it, not before), and fix them one by one. (Here's an example.) Added: We always hear about tradeoffs, right, such ...


55

Most of the focus here seems to be around the word always. Yes, absolutes are bad, and software engineering is almost as much art as it is science, and all that... but I'm going to have to say that for the example you gave, the method would be better if it was split up. These are the arguments I'd typically use to justify splitting up your method: ...


51

Inner functions are not an anti-pattern, they are a feature. If it doesn't make sense to move the inner functions outside, then by all means, don't. On the other hand, it would be a good idea to move them outside so you can unit test them easier. (I don't know if any framework lets you test inner functions.) When you have a function with 250+ lines, and ...


50

There is no definitive naming convention in PHP, and they differ by framework: Zend does not permit underscores Symfony also encourages camelCase Wordpress encourages underscores and does not like camelCase CodeIgniter also promotes underscores So: Use whatever your framework uses or create your own naming convention. At least for function names and ...


50

I would take a step back here. You're concentrating on the picky details of the code but missing the larger picture. Let's take a look at one of your example loops: int offset = 0; while(true) { Record r = Read(offset); if(r == null) { break; } // do work offset++; } What is the meaning of this code? The meaning is "do some ...


50

On the other hand, the more processing you put on a line, the more logic you get on one page, which enhances readability. I utterly disagree with this. Just looking at your two code examples calls this out as incorrect: var a = F(G1(H1(b1), H2(b2)), G2(c1)); is heard to read. "Readability" does not mean information density; it means "easy to read, ...


47

"Simple" is an overused word. "Readable" can profitably be defined as "simple to understand", in which case increasing (this measure of) simplicity by definition increases readability, but I don't think this is what you mean. I've written about this elsewhere, but generally something can be called "simpler" either by being more abstract (in which case fewer ...


44

I would say that it's an acceptable practice. This is a rare instance where I would consider the majority to be wrong and in need of updating their knowledge of recent programming ideas. In many languages, particularly ML-based functional languages like Haskell and OCaml, it is extremely common to use _ as an "unused" variable. Even Lua, which doesn't ...


Only top voted, non community-wiki answers of a minimum length are eligible