284

This would constitute a feature known as a time bomb. DON'T CREATE TIME BOMBS. Code, no matter how well you structure and document it, will turn into an ill-understood near-mythical black box if it lives beyond a certain age. The last thing anyone in the future needs is yet another strange failure mode that catches them totally by surprise, at the worst ...


277

Protected variables should be avoided because: They tend to lead to YAGNI issues. Unless you have a descendant class that actually does stuff with the protected member, make it private. They tend to lead to LSP issues. Protected variables generally have some intrinsic invariance associated with them (or else they'd be public). Inheritors then need to ...


223

There are other problems Neither code is good, because both basically bloat the code with a debug test case. What if you want to test more things for whatever reason? phoneNumber = DEV_PHONE_NUMBER_WHICH_CAUSED_PROBLEMS_FOR_CUSTOMERS; or phoneNumber = DEV_PHONE_NUMBER_FROM_OTHER_COUNTRY; Do you want to add even more branches? The significant problem is ...


219

Most of your reasons to keep it are utterly irrelevant, put simply. If the code isn't used, throw it away- any benefit involved in keeping it can be trivially derived from source control. At most, leave a comment saying which revision to find it in. Quite simply, the sooner you cut the code, the sooner you don't have to waste time maintaining it, compiling ...


216

In layman's words: The important thing is not the numbers of lines but the readability of the code. Any fool can write code that a computer can understand. Good programmers write code that humans can understand. (M. Fowler) In the examples you gave, the second one is definitively easier to read. Source code is for people to read. Besides, ...


215

Taking the code examples first. You favour: if (isApplicationInProduction(headers)) { phoneNumber = headers.resourceId; } else { phoneNumber = DEV_PHONE_NUMBER; } function isApplicationInProduction(headers) { return _.has(headers, 'resourceId'); } And your boss would write it as: // Take the right resourceId if application is in production ...


191

Whilst many, including "Uncle Bob", advise not to use I as a prefix for interfaces, doing so is a well-established tradition with C#. In general terms, it should be avoided. But if you are writing C#, you really should follow that language's conventions and use it. Not doing so will cause huge confusion with anyone else familiar with C# who tries to read ...


163

To answer your question about extant research But has anything been written or researched on recognizing the point where striving for code brevity stops being useful and becomes a barrier to comprehension? Yes, there has been work in this area. To get an understanding of this stuff, you have to find a way to compute a metric so that comparisons can be ...


163

This is a pretty common problem for scientists. I've seen it a lot, and it always stems by the fact that programming is something you pick on the side as a tool to do your job. So your scripts are a mess. I'm going to go against common sense and say that, assuming you're programming alone, this is not so bad! You're never going to touch most of what you ...


158

The principles stated in "Clean Code" are not always generally agreed upon. Most of it is common sense, but some of the author's opinions are rather controversial and not shared by everybody. In particular, the preference for short methods is not agreed on by everybody. If the code in a longer method is not repeated elsewhere, extracting some of it to a ...


155

Yes, it's an acceptable byproduct, and the justification is that it is now structured such that you don't have to read most of the code most of the time. Instead of reading a 30-line function every single time you are making a change, you are reading a 5-line function to get the overall flow, and maybe a couple of the helper functions if your change touches ...


141

Physicist here. Been there. I would argue that your problem is not about the choice of tools or programming paradigms (unit testing, OOP, whatever). It's about the attitude, the mindset. The fact the your variable names are well chosen at first and end up being crap is revealing enough. If you think of your code as “run once, then throw away”, then it will ...


134

You have an invariant: Only a single view (out of 3) is ever active (and visible). Then, I suggest that you provide a function to switch the activity and visibility of ALL views at once: [setActiveView viewID:2] This function will: check if the view is already active, avoiding unnecessary work set the view as active, and visible set the other 2 views ...


130

... we are a very small team supporting a relatively large and undocumented code base (that we inherited), so some developers/managers see value in writing less code to get things done so that we have less code to maintain These folk have correctly identified something: they want the code to be easier to maintain. Where they've gone wrong though is assuming ...


118

Firstly, magic values are avoided in programming by using variables or constants. CSS does not support variables, so even if magic values were frowned on, you don't have much of a choice (except using a preprocessor as SASS, but you wouldn't do that for a single snippet). Secondly, values might not be as magic in a domain specific language like CSS. In ...


116

As others have said, there's a difference between API-documenting comments and in-line comments. From my perspective, the main difference is that an in-line comment is read alongside the code, whereas a documentation comment is read alongside the signature of whatever you're commenting. Given this, we can apply the same DRY principle. Is the comment saying ...


114

It's turtles all the way down. Or abstractions in this case. Good practice coding is something that can be infinitely applied, and at some point you're abstracting for the sake of abstracting, which means you've taken it too far. Finding that line is not something that's easy to put into a rule of thumb, as it very much depends on your environment. For ...


113

I don't share your opinion. In my opinion using global variables is a worse practice than more parameters irrespective of the qualities you described. My reasoning is that more parameters may make a method more difficult to understand, but global variables can cause many problems for the code including poor testability, concurrency bugs, and tight coupling. ...


112

In my perfect fantasy world where I have 100% unit test coverage I would just delete it, run my unit tests, and when no test turns red, I commit it. But unfortunately I have to wake up every morning and face the harsh reality where lots of code either has no unit tests or when they are there can not be trusted to really cover every possible edge case. So I ...


110

You should strive to become irreplaceable not by writing code noone else understands, but by gathering more experience and knowledge than others. The former way makes you a developer everyone tries to avoid working with, as they will fear and loath maintaining code you wrote. The latter way you become a sought out team member, whom managers want to have in ...


104

These guidelines are a compass, not a map. They point you in a sensible direction. But they can't really tell you in absolute terms which solution is “best”. At some point, you need to stop walking into the direction your compass is pointing, because you have arrived at your destination. Clean Code encourages you to divide your code into very small, obvious ...


104

The advantage is that without dependency injection, your Profile class needs to know how to create a Settings object (violates Single Responsibility Principle) Always creates its Settings object the same way (creates a tight coupling between the two) But with dependency injection The logic for creating Settings objects is somewhere else It's easy to use ...


102

It depends, and your example is not useful in making the decision. While fewer lines of code are not always better (at some point it leads to obfuscation), they usually are, simply because there's fewer things to keep track of when trying to understand the code. In your specific example: If the names of the intermediate values actually convey meaning that ...


102

The Robert C. Martin quote is taken out of context. Here is the quote with a bit more context: Nothing can be quite so helpful as a well-placed comment. Nothing can clutter up a module more than frivolous dogmatic comments. Nothing can be quite so damaging as an old crufty comment that propagates lies and misinformation. Comments are not like ...


87

Feature envy is a term used to describe a situation in which one object gets at the fields of another object in order to perform some sort of computation or make a decision, rather than asking the object to do the computation itself. As a trivial example, consider a class representing a rectangle. The user of the rectangle may need to know its area. The ...


84

There are two halves to this process. The first is confirming that the code is indeed dead. The second is comprehending the costs of being wrong and making sure they are properly mitigated. Many answers here have great solutions to the former half. Tools like static analyzers are great for identifying dead code. grep can be your friend in some cases. ...


84

Good answer here already, but let me say a word about your butterknife example: though I have no idea what the code does, at a first glance, it does not look really unmaintainable to me. Variables and method names seem to be chosen deliberately, the code is properly indented and formatted, it has some comments and the long methods at least show some block ...


82

Ideally I think you should extract your logic for getting the alert code/number into its own method. So your existing code is reduced all the way down to { addAlert(GetConditionCode()); } and you have GetConditionCode() encapsulate the logic for checking conditions. Maybe also better to use an Enum than a magic number. private AlertCode ...


82

Version control is probably going to give you the most bang for your buck. It isn't only for long-term storage, it's great for tracking your short-term experimentation and going back to the last version that worked, keeping notes along the way. Next most useful are unit tests. The thing about unit tests is even code bases with millions of lines of code ...


81

It's acceptable because these formats are not code, but data. If you were to remove all the "magic numbers," you would essentially duplicate every label, and end up with ridiculous looking files like: mainkite_width = 200px ... .mainkite { width: mainkite_width; ... Every time you needed to change some data, you would need to look in two places. ...


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