230

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.


179

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 ...


114

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 ...


94

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 ...


90

... 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 ...


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 ...


83

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.


74

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 ...


65

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 ...


62

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 ...


62

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,...


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.


56

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 ...


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

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, ...


49

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 ...


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 ...


44

If this is the only place these predicate functions would be used, you can also use local bool variables instead: private static bool ContextMatchesProp(CurrentSearchContext context, TValToMatch propVal) { bool matchesDefinitionId = (propVal.PropertyId == context.Definition.Id); bool matchesParentId = (!repo.ParentId.HasValue || repo.ParentId == ...


43

I think this advice came from a software metric which called Cyclomatic complexity or Conditional complexity check this wiki page Definition: The cyclomatic complexity of a section of source code is the count of the number of linearly independent paths through the source code. For instance, if the source code contained no decision points such as IF ...


42

In general, the latter is preferred. It makes the call site more reusable. It supports DRY (meaning you have less places to change when the criteria change, and can do it more reliably). And very often those sub-criteria are things that will be reused independently elsewhere, allowing you to do that. Oh, and it makes this stuff a lot easier to unit test, ...


41

I'd put the fluent api to it's own "builder" class seperate from the object it is creating. That way, if the client doesn't want to use the fluent api you can still use it manually and it doesn't pollute the domain object (adhering to single responsibility principle). In this case the following would be created: Car which is the domain object CarBuilder ...


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