38

Are there any techniques in programming that you find to be overused (I.E. used way more excessively than what they should be) or abused, or used a bit for everything, while not being a really good solution to many of the problems which people attempt to solve with it.

It could be regular expressions, some kind of design pattern or maybe an algorithm, or something completely different. Maybe you think people abuse multiple inheritance etc.

Anto
  • 11,157
  • 13
  • 67
  • 103

19 Answers19

72

Comments in Code

I just wish the University Professors would understand they don't need to teach their students to write 10 lines of comments saying the following code is a for loop, iterating from 1 to number of x. I can see that in the code!

Teach them to write self-documenting code first then appropriate commenting on what cannot be adequately self-documenting second. The software world would be a better place.

Dan McGrath
  • 11,163
  • 6
  • 55
  • 81
  • 26
    Self-documenting code isn't . Also professors do this to teach students to conceptualize the problem and how to approach it. Additionally I have never seen anyone complain about too much documentation. Caveat being the documentation is correct. – Woot4Moo Jan 11 '11 at 21:04
  • 24
    @Woot4Moo: if you read `Clean Code`, Fowler clearly states that out-of-date documentation is worse than no documentation, and that all comments have a tendency to become stale and to migrate away from the code they document. That whole book is all about how to write self-documenting code. – Scott Whitlock Jan 11 '11 at 21:35
  • 12
    Teaching them to *replace* comments with code would be a good start... – Shog9 Jan 11 '11 at 21:37
  • 15
    +1. I've actually seen the following in real world code: "loopVar++; // increment loopVar". AAAAAAAAAAAAARRRRRGH! – Bobby Tables Jan 11 '11 at 21:38
  • 2
    @Guzica - Eeeeeewwwwwwwwww. – Craige Jan 11 '11 at 21:45
  • 4
    The mantra of self-documenting code is overused. Programmers are usually not the users of the program they create, as such they often write code about processes they only have a passing familiarity with. Taking the time to explain "why" something is done, rather than just "how" is invaluable for later programmers. Hiding behind "self-documenting" code is often an excuse to be lazy. – Bill Jan 11 '11 at 21:47
  • 7
    @Bill: My take on it is that you shouldn't need to document generic logic and control structures, even if they are relatively complex. Basically, anything that a good developer who knows the language should be able to work out by analysing the code shouldn't need to be commented. eg. A set of nested for loops with some breaks inside them, etc. But what SHOULD be commented is the application-specific reasoning of why the code makes those decisions: eg. "If a new programmer starts at the company tomorrow and looks at the code, will what it does make sense without the comment hints?" – Bobby Tables Jan 11 '11 at 21:56
  • That's not the point I was making though. I totally agree 'why' is important, that is the second part I was attempting to imply with 'cannot be adequately self-documented'. My issue is with people who write comments with that have the same benefit as a turnip ASCII art picture in the code. I don't think anyone should hide behind 'self-documenting' code, I just believe that is how it should be written full stop, no excuse. Whatever is left to be commented such as why & business rules it is implemented are all I should read as comments. Not paraphrasing the sytnax I can already read... – Dan McGrath Jan 11 '11 at 22:02
  • 4
    @Guzica I agree generic stuff should not be commented. But just as there is good coding and bad coding, there is also good commenting and bad commenting. Complex systems require commenting. For example, I work in the ISP world, where we often need to use a lot of complex regex's against a bazillion network devices. Putting in an example of the output being regexe'd against improves understanding of the code. – Bill Jan 11 '11 at 22:09
  • 3
    @Bill: Yeah, that's a good example of what SHOULD be commented. :) – Bobby Tables Jan 11 '11 at 22:12
  • @Scott if you see my caveat I address your issue. – Woot4Moo Jan 11 '11 at 22:15
  • 3
    Self-documenting code...Programmer religion flavor of the week. – Robert Harvey Jan 12 '11 at 03:11
  • 2
    @Robert - I aim to write self-documenting code. I also write a lot of Doxygen annotations, a few other comments, and a fair bit of separate documentation. That's not hypocrisy, but I do think some anti-self-documenting-code claims recently are misrepresenting what it means. Obviously you can't document everything by e.g. choosing clear identifier names, but that doesn't mean there's no value in doing so, and by avoiding the need for many comments that way you get the opportunity to add comments for other things without creating excessive clutter. –  Jan 12 '11 at 03:17
  • 1
    @Scott - the copy of `Clean Code` on my desk was written by Robert C. "Uncle Bob" Martin, although I'm sure Fowler (I assume you meant Martin?) would be pretty much in agreement. ;-) – Mike Woodhouse Jan 12 '11 at 09:17
  • 1
    @Bill - for the example you provide (regex sample ins/outs) would it not be even better to provide the documentation in executable unit tests? If you really need to spoon-feed the reader, the comment might then simply direct them to those tests. – Mike Woodhouse Jan 12 '11 at 09:19
  • 1
    If you have to explain the "how" in your code, you're implying that it's not clear by looking at the code what it does. REWRITE IT! Comments should only explain why not how. – Neil Jan 12 '11 at 12:00
  • 2
    I don't understand people who don't update their comments when they modify a function: you wouldn't leave the old version of the code when you modify a function, so why stop halfway ? This kind of lazyness makes me want to shake the stupid of the people who do it. – wildpeaks Jan 12 '11 at 13:02
  • 2
    @Steve314: I am in favor of writing code that is self-describing. I am also in favor of adding comments when it is useful to do so. I like syrup on my waffles, but not if it overflows the plate. – Robert Harvey Jan 12 '11 at 20:15
  • 1
    If you have a source code indexing system, like `lxr` or `grok`, try searching for some rude or swear words. Are they in the code or in the comments? While at Microsoft, we found an awesome programmer that wrote lots of comments but unfortunately just did not know the correct spelling of *couldn't*. – JBRWilkinson Jan 13 '11 at 17:48
  • 4
    When I was at Uni they used auto-marking a lot (mainly in the first year), including points for "commenting your code". But it only checked for the existence of a comment. We always used to put things like `// This comment doesn't explain any of my simple code but has be here to get the marks` – DisgruntledGoat Jan 14 '11 at 13:21
  • @user9094 - what if you don't fully understand the comment? By changing it, you could lose information that someone else might benefit from. As you're busy at the moment, you may think, and you don't want to interrupt your current flow, you'll leave that for a bit and get back to it later... but later may be much later than you intended. –  Jan 16 '11 at 22:01
  • @Mike Woodhouse - yeah, but I can't edit it now... – Scott Whitlock Jan 17 '11 at 18:59
56

The singleton design pattern.

Sure, it's a useful pattern but every time a new programmer learns about this pattern he starts trying to apply it on every single class he creates.

Raphael
  • 2,234
  • 1
  • 22
  • 23
  • I use this poorly designed excuse for a framework every day. http://codeigniter.com/user_guide/libraries/email.html They think OOP is prefixing functions with `$this->`. PHP is a rotten language so I can't expect the frameworks to be excellent. – Benbob Jan 11 '11 at 23:14
  • Yeah, this is a pattern that every shitty programmer will bring up during the interview, because that's the only one he knows (but probably still doesn't understand). – gruszczy Jan 11 '11 at 23:25
  • @Keyo: Have you tried Zend Framework instead, or do your overlords insist on CodeIgniter? – Orbling Jan 12 '11 at 03:31
  • @Keyo: A singleton is not a super object, which you would understand if you knew what you were talking about. – Josh K Jan 12 '11 at 03:37
  • @Keyo: You also don't seem to understand how that object is being built, as the libraries aren't functions with a `$this->` prefix. – Josh K Jan 12 '11 at 03:39
  • 7
    I think any programmer who implements a singleton and does not repent will burn in a fiery hell for the sin(s) he (or she) has committed. –  Jan 12 '11 at 06:27
  • 2
    I implemented a singleton once in my professional career (in a multitasking context) and I had to repent for my sin by rewriting it. – Spoike Jan 12 '11 at 09:31
  • 3
    Singletons can be useful for a few things, but are almost always used when they shouldn't be. That doesn't mean singletons should never be used - just because something is abused doesn't mean it can't be used correctly. – configurator Aug 31 '11 at 16:21
  • @configurator - Agreed. It's just that usually when a developer discovers this pattern he(she) will likely try to apply it everywhere (I must admit that I've also committed this sin when I first studied design patterns). – Raphael Aug 31 '11 at 17:24
  • 2
    @Rapahel: That's exactly why I don't believe studying design patterns is a good thing. – configurator Sep 01 '11 at 12:58
53

The most evil thing to protect dumb programming. putting everything in a try catch and doing nothing with the catch

        try
        {
            int total = 1;
            int avg = 0;
            var answer = total/avg;
        }
        catch (Exception)
        {

        }

I shudder when I see a try catch that does nothing and is designed to handle dumb logic. In general try catches are over used, however in some cases are very useful.

mu is too short
  • 177
  • 2
  • 7
Nickz
  • 1,430
  • 1
  • 13
  • 18
  • 4
    Funny thing - I've just been editing a file that does that hundreds of times, and with good reason. It's a unit test file, testing that various methods throw an exception when expected. The try block contains a call and an "expected throw didn't happen" report. The exception is expected and correct, so there is no corrective action. Obviously unit tests are a special case, but I have had similar cases in real code. Red flag, yes, but not an absolute rule. –  Jan 12 '11 at 02:44
  • 5
    @Steve the case you described is an rare edge condition. Another one I can think of is logging code -- if logging itself fails, there's no point trying to log the exception, or interrupting the app. – dbkk Jan 12 '11 at 06:19
  • 1
    @dbkk - agreed on the edge condition, and I'd add that when you genuinely need to catch an exception but don't need a corrective action, adding a comment in the catch block to explain that is essential if only to calm down the maintainers. It's not so rare in unit tests, so I'd drop the comment thing in that case. –  Jan 12 '11 at 11:51
  • 5
    on error resume next – jk. Jan 12 '11 at 13:26
  • I agree if logging errors code throws an except what can you do but silently handle it. – Nickz Jan 13 '11 at 05:44
  • That's where things like Python's `assertThrows` or some such is great. – l0b0 Jan 13 '11 at 17:07
  • I wound up once having an empty exception block for what at least looked like a good reason at the time. I commented it to explain what was going on. Anytime I do something that's usually done for dumb reasons, I feel compelled to comment it. – David Thornley Jan 13 '11 at 19:31
  • There is a huge difference between a catch that catches ALL exceptions, and a catch that catches A SPECIFIC exception. The poster is suggesting that catching ALL exceptions is probably not a good idea. – Frank Shearar Jan 16 '11 at 12:17
  • 2
    @jk01 - an exception isn't (necessarily) an error. Whether any condition is an error or not depends on context. For example, "file not found" isn't an error if you don't need the file. Perhaps it's just an optional settings file that may or may not be present, and you can just leave your defaults in place if the file isn't there (so no corrective action is needed after the catch). –  Jan 16 '11 at 19:16
  • 1
    I think this is just a symptom showing that checked exceptions aren't a good idea :) – zvrba Jan 19 '11 at 08:21
  • Ruby code uses this technique to try and load a better implementation of X and, if the better implementation is unavailable, later code will simply use the fallback implementation. – yfeldblum Jan 19 '11 at 17:19
  • 1
    @zvrba If the exception was unchecked, the source wouldn't make it glaringly obvious that the programmer didn't know what they were doing. – Tom Hawtin - tackline Jan 20 '11 at 00:42
  • 1
    @Tom: but at least the program would visibly crash. This way it will silently deliver nonsense results. Now, which is easier to debug? – zvrba Jan 20 '11 at 08:19
  • 1
    @zvrba Well one way you don't even need to run the program to see it is utterly broken. The other you have to get lucky and see the *exceptional* circumstance on your development machine. One of these wins for me. (I'm perhaps a little on the extreme end of the spectrum - my job has involved pressing Page Down an awful lot.) – Tom Hawtin - tackline Jan 20 '11 at 16:04
  • 1
    This is why I despise API's that throw exceptions to return errors, like much of Microsoft's file operations. If I can't open a file or delete a file or whatnot, I don't need an exception thrown that forces me to clutter otherwise clean code with exception handlers. I simply report an error and continue on my merry way. A simple return result is far better and cleaner. Thus, the reason for the global catch. I could see your gripe as legitimate if the only time exceptions were thrown was because something truly exceptional happened, but that isn't generally true. – Dunk Aug 31 '11 at 21:40
47

Reliance on StackOverflow to solve your problems instead of figuring it out the hard way.

New programmers that find the wealth of knowledge on SO (too often) post before thinking and trying stuff out.

In my day we had to code from books, no internet, no SO. Now get off my lawn.

Byron Whitlock
  • 329
  • 2
  • 7
  • 29
    I must admit, I do flinch at the audacity of some questions on SO. Either because they are asked many times every day and they have obviously not searched at all; or worse still, they treat the community as a free outsourcing service. – Orbling Jan 12 '11 at 03:27
  • @Orbling Some problems are more difficult than it appears at first. The algorithm itself may seem straightforward, but real-world handling of edge cases (some of which often don't come to mind) is often significant. That's where community helps, even with simple issues. – dbkk Jan 12 '11 at 06:22
  • Just wondering, what do you think of [my questions](http://stackoverflow.com/users/34537/acidzombie24) –  Jan 12 '11 at 06:30
  • 2
    @dbkk: http://stackoverflow.com/questions/4665778/copy-content-of-one-stl-container-to-another --> first comment "did you ever try this ?" – Matthieu M. Jan 12 '11 at 07:44
  • 5
    I started learning from books but once I had a fundament I learned almost everything else from online forum discussions. Not by asking, but mainly (and at first, exclusively) by reading. This method has taught me several programming languages in excruciating depth (with lots of nooks and crannies), and I think it’s not a bad method at all. – Konrad Rudolph Jan 12 '11 at 08:54
  • 1
    @Konrad Rudolph: That method is good and wise, reading questions, searching well. The asking is best done when the asker has a framework with which to understand the answers. Too often people ask questions which they do not (yet) have the capacity to understand. – Orbling Jan 12 '11 at 12:41
  • @acidzombie24: You certainly have a lot of questions! Some are excellent, simple things, but very important that provide great question/answer sets for future readers. Some are a bit cheaper, but at least all your questions make sense and have some point to them. – Orbling Jan 12 '11 at 12:43
  • @dbkk: Yes, there are simple questions with complicated answers due to edge cases and other hurdles. I meant people asking questions that are either so regular as to be impossible to miss if they had tried other avenues to answer, and the people who essentially give a brief, or put a ton of code on the screen and say "please fix it". – Orbling Jan 12 '11 at 12:45
  • LOL - I hate that so much and the funny thing is I get crap for telling people they could have done a little research and found their answer all the time. – Edward Strange Jan 13 '11 at 08:23
  • No way, where I work this is chronically underused. I routinely send links to my coworkers to questions I hear them muttering to themselves that I ask on SO. And wow, I am in awe the man who contributed to 1% of SO. – Peter Turner Jan 13 '11 at 18:41
  • 3
    the problem is that SO encourage this by giving a lot of rep for asking/answering very easy question that can be answered with simple google search. very hard question people answer only because they really want to. – IAdapter Jan 14 '11 at 10:37
  • 1
    And that's exactly the problem with this kind of site. It encourages answering easy questions or giving the "populist" answer that will get you the most votes. I have personally seen (and had happen to me) the best answer downvoted or not given upvotes while a one-liner or ridiculous answer gets upvoted and marked as correct, when it's **wrong**. – Wayne Molina Aug 31 '11 at 19:48
  • I thought your answer had some merit until I read "In my day..." At the time of posting this comment, we are in 2015. Why can't you just accept it and move on from back in your day? Next I suppose you'll think everyone should learn Assembly (which could actually be good in some ways, but that's beside the point since it is impractical) – brettwhiteman Mar 15 '15 at 06:11
36

OOP evidently. It is very valuable, but when misused it can obscure otherwise clear code pretty easily (some examples of S4 programming in R come to mind), and can give a tremendous overhead that is unnecessary and can cost you big time on performance.

Another thing is that (in eg Perl) OOP often comes down to writing the same thing the other way around : result = $object ->function(pars) instead of result = function(object, pars) This sometimes makes sense, but when mixed with procedural programming it can make reading the code pretty confusing. And apart from that, you just introduce more overhead where it doesn't improve the design. It comes down to what is said about the singleton pattern as well: Should be used, but not on everything.

I do use OOP myself, but in my field (scientific computing) performance is a big deal, and OOP is often abused there as all students get Java nowadays. It's great for dealing with larger problems, for conceptualizing and so on. But if you work with eg DNA sequences in BioPerl, using the objects every time and working consistently with getters and setters can increase the calculation time a tenfold. When your code is running a few days instead of a few hours, that difference really matters.

Joris Meys
  • 1,933
  • 14
  • 20
  • @Joris: That's what OOP is though. Calling a method, or messaging an object, instead of calling an arbitrary function and passing it the object it is to operate on. – Josh Jan 11 '11 at 21:07
  • -1 Because I agree with @Josh. I'm not sure you truly understand the purpose of OOP. – Craige Jan 11 '11 at 21:18
  • @Josh @Craige : I guess you missed some important words in my text. I clarified. – Joris Meys Jan 11 '11 at 21:22
  • @Joris Meys - I'll remove the down vote for your edit and clarification, but I still don't believe you fully understand the power of OOP. One great example that cannot be completed with your `fn(ojb, pars)` syntax is method chaining. – Craige Jan 11 '11 at 21:26
  • 6
    @Craige : I do use OOP myself, but in my field (scientific computing) performance is a big deal, and OOP is often abused there as all students get Java nowadays. It's great for dealing with larger problems, for conceptualizing and so on. But if you work with eg DNA sequences in BioPerl, using the objects every time and working consistently with getters and setters can increase the calculation time a tenfold. When your code is running a few days instead of a few hours, that difference *really* matters. – Joris Meys Jan 11 '11 at 21:35
  • @Joris Meys - Touche. Put that directly in your answer where it can be seen and I'll give you my up-vote. – Craige Jan 11 '11 at 21:37
  • @Craige : did it, thx. – Joris Meys Jan 11 '11 at 21:39
  • 1
    @Craige: Method chaining (like `Foo.DoX().DoY().DoZ()`) is nice, but, but it's definitely not the only way to go about something. Function composition (like `doZ . doY . doX $ foo` is a perfectly valid alternative. – Anon. Jan 11 '11 at 21:39
  • @Anon - indeed it's not the only way. I was just pointing out a single aspect of OOP that is quite useful. – Craige Jan 11 '11 at 21:45
  • @Joris, interesting. Modern JVM's should inline get/set calls transparently. Isn't that the case for the JVM you use? –  Jan 11 '11 at 22:01
  • +1, last vote of the day for an important point. OOP is great, very useful, but not a panacea. – Orbling Jan 11 '11 at 22:01
  • 1
    @Joris: something I have wondered about for a long time (myself a bioinformatician): if performance matters that much, why use Perl in the first place instead of C++? If you’re worried about shaving off a factor 10 from the runtime (valid concern!) then switching to C++ gives you instant gratification. Of course, the language sucks … but then, so does Perl … and there are good bioinformatics libraries for C++. – Konrad Rudolph Jan 12 '11 at 08:48
  • 1
    @Konrad : It depends. BioPerl has arguably the most packages for bioinformaticians from all languages (I reckon Python is catching up though). Plus, a lot of work has been done in Perl and adapting a script is more easy than rewriting everything in C++ . Lastly, it doesn't serve to write a complete application when a script will do. Guess that's why Perl is still around that much. And honestly, I don't mind, I like the language. It's still the best I know for string and file work. The calculations are obviously done in other languages and linked back (which goes pretty easy). – Joris Meys Jan 12 '11 at 10:10
  • @Joris: ah, didn’t know that the actual calculations were done elsewhere (I’ve never used BioPerl). I have worked with a few Perl tools (e.g. the vcake assembler) that *are* written completely in Perl, and run appropriately slowly. – Konrad Rudolph Jan 12 '11 at 10:14
  • 1
    @Konrad : it depends on how one does it. It is possible to do all in Perl, but Perl can be easily linked to other tools, especially on Linux. I use Perl often as an advanced bash-script with strong string-manipulation and easy construction of datasets to be dropped into other applications. – Joris Meys Jan 12 '11 at 11:38
  • For writing the same thing pythons ', '.join(list) comes to mind. list.join(',') makes so much more sense to me... – Inca Jan 16 '11 at 09:58
  • The problem with getters/setters changing performance is a sign of a crappy implementation, not OOP. C++ compilers, for example, can trivially remove them. – DeadMG Aug 31 '11 at 17:12
  • @DeadMG : R is an interpreted language and does create more overhead when using the S4 OOP system. Alas, not every programming language is C++. So it is possible to abuse it. Mind the word choice: ABuse != use. I'm talking about abuse here, not use. As you might have noticed, I did say that OOP can be a very strong concept when used in the right place at the right time. I use it often enough myself. – Joris Meys Sep 02 '11 at 14:06
  • @KonradRudolph Please keep your opinion of languages to yourself. – brettwhiteman Mar 15 '15 at 06:15
  • @Brett You’ve missed the gist of my comment. The “opinion of languages” is irrelevant. – Konrad Rudolph Mar 15 '15 at 10:15
27

Having spent several years in ASP.NET Webforms, i'd have to say unequivocally:

The Smart UI

While it's entirely possible to create layered, testable applications in webforms, Visual Studio makes it too easy for developers to one-click their way to forms tightly bound to their data-source, with application logic littered around all the UI code.

Steve Sanderson explains this anti-pattern much better than I could in his Pro ASP.NET MVC book:

To build a Smart UI application, a developer first constructs a UI,usually by dragging a series of UI widgets onto a canvas, and then fills in event handler code for each possible button click or other UI event. All application logic resides in these event handlers: logic to accept and validate user input, to perform data access and storage, and to provide feedback by updating the UI. The whole application consists of these event handlers. Essentially, this is what tends to come out by default when you put a novice in front of Visual Studio. In this design, there’s no separation of concerns whatsoever. Everything is fused together, arranged only in terms of the different UI events that may occur. When logic or business rules need to be applied in more than one handler, the code is usually copied and pasted, or certain randomly chosen segments are factored out into static utility classes. For so many obvious reasons, this kind of design pattern is often called an anti-pattern.

richeym
  • 3,007
  • 3
  • 22
  • 23
  • 1
    A GUI is at least some sort of specification code must adhere to. I don't think the programmers he describes would do any better if presented with a blank text file instead. –  Jan 12 '11 at 13:41
  • 4
    @qpr, I don't know about that. If they had a blank text file they might not be able to do anything (which might just be a bonus). – Ken Henderson Jan 12 '11 at 14:05
  • One of the many benefits of using WPF is that implementing MVVM smashes this anti-pattern into smithereens. – Robert Rossney Jan 12 '11 at 18:55
  • 2
    A thousand times this. I even see experienced .NET developers relying on this "pattern" because it's easier than understanding or caring about Separation of Concerns. Even if they don't use the drag-and-drop wizards, the most common "pattern" in .NET WebForms development is to do everything in code-behind events and copy/paste code as needed. – Wayne Molina Aug 31 '11 at 16:22
23

I see this occasionally and it usually results from not fully understanding boolean expressions.

if (someCondition == true)
Corey
  • 1,812
  • 1
  • 15
  • 18
  • 16
    It does have the advantage of making code more explicit – Pemdas Jan 11 '11 at 21:03
  • 1
    @Pemdas - Agreed – Craige Jan 11 '11 at 21:16
  • 7
    I think a look at this question is in order: http://programmers.stackexchange.com/questions/12807/make-a-big-deal-out-of-true – gablin Jan 11 '11 at 21:17
  • 5
    I don't do that but, seriously, get over it. There's a hell of a lot worse out there. :) – MetalMikester Jan 11 '11 at 22:56
  • I changed an instance of this and my lead, who I respect as a brilliant programmer, asked me to change it back as it was more explicit. – Dominique McDonnell Jan 12 '11 at 02:22
  • 1
    @Pemdas: disagree. If anything it makes the code *less* explicit since now it looks as though we’re testing for an arbitrary equality relation instead of for a condition. – Which, strictly speaking, is also an equality test but it’s a special case that’s important enough to merit an own syntax. – Konrad Rudolph Jan 12 '11 at 08:46
  • 4
    If you named your variables properly, for example `bool` s beginning with `is` or `has`, you don't need to make your code more explicit with `= true`. – Nobody Jan 12 '11 at 10:52
  • 1
    I've found this to be a strong code smell, not just because it's poor practice in itself, but because it's a strong indicator that the developer doesn't understand the language properly. For example, one dev I worked with who consistently wrote conditions this way in C# also insisted on passing lists by `ref` if the list was going to be modified. He clearly didn't understand the way memory was used, and he wrote a lot of nasty code as a consequence. – Bevan Jan 13 '11 at 20:07
  • If you only "see this occasionally", how is it *overused*? – DisgruntledGoat Jan 14 '11 at 13:23
  • 3
    @DisgruntledGoat because it shouldn't be in use at all – Corey Jan 14 '11 at 15:59
  • What makes this anti-pattern evil is that some yokel comes along and changes it to `if (someCondition != true)` which is a 10' speed bump with respect to readability. – Thomas Mar 20 '11 at 02:34
  • I see this used quite often, much to my chagrin... from senior developers with several years experience... – Wayne Molina Aug 31 '11 at 19:56
  • This is terrible! To make it more explicit, we must check for true at least 3 times: if (((someCondition == true) == true) == true). This is great, as no one can misunderstand! If anyone does, just add more checks for true. There is no limit! :) – Frank Hileman Sep 04 '14 at 15:08
19

Reimplementing core (or otherwise provided) functionality, either because of ignorance of its existence or because of a perceived need for customizations.

l0b0
  • 11,014
  • 2
  • 43
  • 47
  • 3
    Learning is an exception. When studying anything, it's often beneficial to "reinvent the wheel." – Maxpm Jan 19 '11 at 05:17
  • For sure - I should have specified that it's relevant only when creating production software. – l0b0 Jan 19 '11 at 08:02
  • 1
    This is particularly bad when implementing anything related to security. Good security software prevents attacks that most people never even think of. – David Thornley Jan 19 '11 at 20:44
  • I see this way too common. We need X, let's write our own! But what about open-source package Y? No, we need custom software for our super-secret trade secrets that are the same as everyone else in our industry, we can't use common rubbish software! – Wayne Molina Aug 31 '11 at 20:08
18

Inheritance.

Don't enforce is-a where it doesn't make sense.

Kit
  • 193
  • 1
  • 5
  • 6
    The problem is that intuitively “is-a” appears very often to make sense (in particular since every implementation/contract relationship can colloquially be expressed as “is-a”). It requires a solid understanding of OOP to realize that this is in fact a mistake, and that inheritance and interface implementation are fundamentally different. – Konrad Rudolph Jan 12 '11 at 08:56
  • @Konrad - agree absolutely. I try to encourage people to [start with composition](http://www.artima.com/lejava/articles/designprinciples4.html), move on to interfaces and consider inheritance last. (As a rule of thumb, of course). But maybe that's my VB background talking... ;-) – Mike Woodhouse Jan 12 '11 at 09:26
  • 1
    This is largely a language design question, though. The reason why you should "prefer composition over (implementation) inheritance" in languages like Java or C#, is because implementation inheritance sucks in those languages. Some people criticize Bertrand Meyer's *Object-Oriented Software Construction* for overuse of inheritance, but when you actually look at Eiffel (the language used in the book), you realize that it was *designed for implementation inheritance*, and thus it's actually okay to use inheritance over composition. In *that* case, at least. – Jörg W Mittag Jan 16 '11 at 08:08
14

Customizing off-the-shelf software.

While I can acknowledge this can be useful, I do wonder how much money is spent on getting little things done for questionable gains. This is where a company may spend hundreds of thousands if not millions of dollars on licenses for some software that then gets customized because it is so configurable, editable, and flexible that it really doesn't do much by default. In the end it is a custom application because of all the new code added what was initially bought.

JB King
  • 16,795
  • 1
  • 40
  • 76
13

Using the complier as a debugger.

Ignoring complier warnings or turning them off completely.

Global variables.

Pemdas
  • 5,385
  • 3
  • 21
  • 41
  • 18
    What's wrong with "using the compiler as a debugger"? Having a compiler that's able to point out errors in your code before they become an issue at runtime is a tremendous advantage. – Mason Wheeler Jan 11 '11 at 21:00
  • Because people start writing crappier code knowing that the compiler will catch their mistakes. – Pemdas Jan 11 '11 at 21:01
  • 3
    I rely on the compiler catching my syntax errors and incorrect type-matching mistakes. For behavioural errors, I rely on test cases. – gablin Jan 11 '11 at 21:16
  • 1
    huh? if the code doesn't compile, then they'll have to re-write it. I'm not sure what you're moaning about. – Matt Ellen Jan 11 '11 at 21:20
  • 5
    The problem is when you start to depend on the compiler to ensure program correctness. Did this compile? No, let me munge this little section and see if that fixed it. Did it compile? No, let me add a * here and there and see if compiles. ...ect. Obviously, it useful for correcting typos – Pemdas Jan 11 '11 at 21:23
  • 2
    You say that as if the coder is poking around blindly in the dark trying to do something--anything--to get the code to somehow compile. That's not how it works IME; the compiler gives you a useful error message and points you at the location of the error, and usually if something's not compiling it's new code that you wrote yourself, so you have a pretty good idea what's wrong and how to fix it once it's been pointed out. (Although if you're tossing random *s around, you might be working in C++, where compiler errors are known to be supremely unhelpful, so what I said may not apply.) – Mason Wheeler Jan 11 '11 at 21:35
  • 2
    While I agree that the compiler does point to errors in the code they are only syntactical errors. An over reliance on such error reporting lulls the programmer into a false sense of validity. They should be treated for what they are a measure of ones typing ability for your language of choice, (fancy spell checker). – Jeff Jan 11 '11 at 21:53
  • 3
    Relying on the compiler works very well, given a strong enough type system and a well-typed code base. Often, the type system *can* model important constraints that you’d otherwise have to write tests for (e.g. in weakly typed systems). So used judiciously, the compiler (and particularly its type checker) is a *great* asset for testing and debugging, and there’s *nothing* wrong with relying on it. In particular, it’s wrong to say that a compiler can only show syntax errors. – Konrad Rudolph Jan 12 '11 at 08:39
  • @Mason: With regard to coders poking around blindly in the dark, I've known some programmers that you evidently have not. There are people who fiddle more or less randomly with code until it compiles, and then think they've accomplished something. You are fortunate in not having encountered that. – David Thornley Jan 13 '11 at 19:34
12

All Design Patterns.

They are like salt or sugar. Some of them on your food make it great, a lot of them, make your food sucks.

Don't get me wrong. Design Patterns are wonderful techniques. I used them a lot. But sometimes, I find programmers, (some of them my ex-bosses), who had these "you have to use a design pattern", even if doesn't match the problem !!!

One example is the "Visitor Pattern" who is more directed for "Lineal / Sequential Collections". I had to work with a tree data structure, once. In order to "visit" each item, I code a non design pattern method, and a former boss insist to use or adapt the "Visitor Pattern". Then, I browse the net, for a second opinion. Well, others programmers had the same situation, and the same solution. And call it the "Tree / Hierarchical Visitor Pattern"., as a NEW Design Pattern

I hope its added as a new pattern to the existing ones, in a new version of the "Design Patterns" book.

umlcat
  • 2,146
  • 11
  • 16
  • 3
    I never use design patterns, although they sometimes emerge naturally in my code. – configurator Aug 31 '11 at 16:31
  • How does the saying go? You start using patterns without knowing you're using them, then you learn about them and use them everywhere, then they become second nature so you starting using them without knowing you're using them? – Wayne Molina Aug 31 '11 at 20:18
  • 2
    @configurator When I read about the design patterns; I also discover that me and others developers where already using some of them ;-) – umlcat Sep 01 '11 at 15:14
9

Using exceptions as flow control. Sort of similar to this answer, but different.

Swallowing exceptions is bad form because it introduces mysterious, hard to find bugs in code. Using exceptions as flow control, on the other hand, is bad because it makes the code far more inefficient than it could be, and is conceptually sloppy.

Exception handling should only be used to handle truly exceptional (duh), unforeseen scenarios, not things like a user typing in an alphabetic character where a number is expected. This kind of exception abuse is extremely common among bad programmers in the Java world.

Bobby Tables
  • 20,516
  • 7
  • 54
  • 79
  • Exceptions making for inefficient code depends on your language - creating a stack frame etc etc. In Squeak, your stack's fully reified, so there's no difference between using exceptions or not. (In other words, exceptions are part of the library, not the language.) However, it is _confusing_ working with exception-controlled control flow: http://www.lshift.net/blog/2011/01/04/try-again-with-exceptions shows a loop-using-exceptions that I recently found. – Frank Shearar Jan 16 '11 at 12:26
  • Regarding the last paragraph, again, it depends on your language. Exceptions ("conditions") in Common Lisp are a strict superset of most languages' ideas of exceptions. See examples here: http://www.gigamonkeys.com/book/beyond-exception-handling-conditions-and-restarts.html As with everything, you can go too far! – Frank Shearar Jan 16 '11 at 12:28
  • Readability and maintainability outweighs performance. The cases where I'll use exceptions for success (what I assume you mean by "flow control") are very rare, but they can give cleaner code in e.g. a complex recursive search. Broadly, I agree though. For example, I have containers with iterator methods that return false for going out of range (common at the end of an iteration), but throw an exception if the iterator is "null" (a probable sign of some kind of internal inconsistency) –  Jan 26 '11 at 15:50
8

I'm going to go with inflexibility through excessive data hiding.

We all know that abstraction and hiding the implementation are good, but more is not always better. Overdone, you can get an inflexible result that can't cope with changes in requirements. To handle the change, you not only have to modify the class that needs to handle that change, but you also have to create a way for the information it never needed before to be accessible despite layers of data hiding.

The trouble is, as with all finding-the-right-balance-for-every-context issues, it takes experience.

7

Mocking. It introduces too many artificial requirements for excessive decoupling into your code, leads to overengineered ravioli code and forces OO-style design down your throat when a more procedural or functional design might be a simpler solution to your problem.

dsimcha
  • 17,224
  • 9
  • 64
  • 81
7

Exposing internal arrays

    public class Data {
    int[] x;

    public void setArray(int[] x) {

        this.x = x;
    }

    public int[] getArray() {

        return x;
    }
}

According to http://www.oracle.com/technetwork/java/seccodeguide-139067.html

copy mutable objects before returning, unless the intention is to share state.

Vivart
  • 369
  • 1
  • 3
  • 10
6

Mutable state and loops. You almost never need them, and you almost always get better code without them.

For example, this is taken directly from a StackOverflow thread:

// ECMAScript
var thing, things_by_type = {};
for (var i = 0; i < things.length; i++) {
    thing = things[i];
    if(things_by_type[thing.type]) {
        things_by_type[thing.type].push(thing);
    } else {
        things_by_type[thing.type] = [thing];
    }
}

# Ruby
things_by_type = {}
things.each do |thing|
  (things_by_type[thing.type] ||= []) << thing
end

They are both doing the same thing. But I have no idea what they are doing. Fortunately, the question actually explains what they are doing, so I was able to rewrite them as follows:

// ECMAScript
things.reduce(function (acc, thing) {
    (acc[thing.type] || (acc[thing.type] = [])).push(thing);
    return acc;
}, {});

# Ruby
things.group_by(&:type)

// Scala
things groupBy(_.type)

// C#
from thing in things group thing by thing.Type // or
things.GroupBy(thing => thing.Type);

There's no loops, and no mutable state. Well, okay, no explicit loops and no loop counters.

The code has become much shorter, much simpler, much more like a description of what the code is supposed to do (especially in the Ruby case it pretty much directly says "group the things by type"), and much less error-prone. There's no danger of running off the end of the array, fencepost errors or off-by-one errors with the loop indices and termination conditions, because there are no loop indices and termination conditions.

Jörg W Mittag
  • 101,921
  • 24
  • 218
  • 318
  • I do believe that in the second line of your "fixed" ECMAScript should read `(acc[thing.type] || (acc[thing.type] = []))`, as opposed to ` = [thing]`, unless you want to add `thing` to the list twice... Or am I missing something? – Austin Hyde Jan 19 '11 at 05:04
  • @Austin Hyde: You're right. – Jörg W Mittag Jan 19 '11 at 15:43
  • 1
    That ecmascript example is incomprehensible to many programmers. It relies on too many syntactic tricks. The loop has the advantage of obviousness for junior developers. – Joeri Sebrechts Aug 31 '11 at 19:07
3

Delphi programmers the world over have been finding out over the past two years how bad it is to use character arrays to store bytes due to the wholescale Unicode conversion

And, "soon" we'll learn how bad it is to store integers in lists when we want to make the change to 64-bits.

Peter Turner
  • 6,897
  • 1
  • 33
  • 57
  • 2
    I think a huge fraction of the Unicode issues would have never come up if Borland had declared a DataString type that was basically the same as AnsiString but specifically for use with blobs, and then made sure people knew about it. – Mason Wheeler Jan 11 '11 at 21:03
2

Properties. I know this is controversial, but I feel that properties are vastly overused in .net.

What's the difference between these two lines?

public string Property { get; set; }

public string Field;

TO the developer, the difference is: absolutely nothing. The compiled form is a tiny bit different, so if you are writing a library, and that library is going to be upgraded in programs, and you can't recompile the programs themselves (for example if you're writing an interface for plugins that should work across versions of your software), then you shouldn't use public fields because you can't replace them with properties. In any other case, there's absolutely no difference between using a field or an auto-property without modifiers.

configurator
  • 2,796
  • 2
  • 21
  • 23
  • 1
    Agreed; **if** you know 100% you will never need to do anything with getting/setting a "property" then there's no reason whatsoever to have a property. However, if there's a chance you will need to do something (logging that a value was changed, for instance) then you should use a property. Personally I don't see a big enough difference to *not* use the property, just in case you **do** need to modify it later (I know, I know `YAGNI` but I feel deviating in this case doesn't cost anything) – Wayne Molina Aug 31 '11 at 20:24
  • 2
    @Wayne: How is changing `;` to `{ get { ... } set { ... } }` any more work than changing `{ get; set; }` to `{ get { ... } set { ... } }`? – configurator Sep 01 '11 at 12:57
  • Now that I think of it, that's a fair point. – Wayne Molina Sep 01 '11 at 13:01