Tag Info

Hot answers tagged

75

I think most answers missed the point here. You're adding multiple lines after method scopes, right? Well that's simply not a common convention. And because it's not a common convention, it's annoying to keep noticing it while reading code. So to be less annoyed: stick to one convention. Even if that's what your teammates want and you don't. Otherwise, ...


65

Be concerned. You write unit tests to prove your code acts the way you expect. They allow for you to refactor quickly with confidence. If your tests are fragile, difficult to understand, or if they are difficult to maintain, you will ignore failing tests or turn them off as your code base evolves, negating many of the benefits of writing tests in the ...


62

A code smell is a symptom which indicates that there is a problem in the design which will potentially increase the number of bugs: this is not the case for regions, but regions can contribute creating code smells, like long methods. Since: An anti-pattern (or antipattern) is a pattern used in social or business operations or software engineering that ...


62

Are unit test smells important? Yes, definitely. However, they are different from code smells because unit tests serve a different purpose and have a different set of tensions that inform their design. Many smells in code don't apply to tests. Given my TDD mentality, I would actually argue that unit test smells are more important than code smells because the ...


42

Definitely not evil and not a code smell in my mind. Data containers are a valid OO citizen. Sometimes you want to encapsulate related information together. It's a lot better to have a method like public void DoStuffWithBottle(Bottle b) { // do something that doesn't modify Bottle, so the method doesn't belong // on that class } than public void ...


41

The problem is getting even more pushing as I'm now working with a team of experienced developers, and sometimes my attempts at writing smart code seem foolish even to myself after time dispels the illusion of elegance. Your solution lies here. I'm presuming that "experienced" in this context means "more experienced than you." At the very least, you ...


35

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


34

First off, I can't stand the term "code smell" anymore. It is used too often and is much of the time thrown about by people who couldn't recognize good code if it bit them. Anyways... I personally don't like using a lot of regions. It makes it harder to get at the code, and the code is what I am interested in. I like regions when I have a large chunk of ...


33

The issue I have seen when maintaining code that makes use of flags is that the number of states grows quickly, and there are almost always unhandled states. One example from my own experience: I was working on some code that had these three flags bool capturing, processing, sending; These three created eight states (actually, there were two other flags ...


32

Here's an example when flags are useful. I have a piece of code which generates passwords (using a cryptographically secure pseudorandom number generator). The caller of the method chooses whether or not the password should contain capital letters, small letters, digits, basic symbols, extended symbols, Greek symbols, Cyrillic ones and unicode. With flags, ...


29

I think we must make a difference between code ownership: from the responsibility point of view, from the code style/hacks etc. point of view. The first one must be encouraged. If nobody is responsible for low quality code and bugs, bad things will happen. It doesn't mean that the bug related to your own code must be assigned every time to you: if the ...


28

Code organization is all about displaying enough information to convey a single idea. The sweet spot is getting your code pared down enough that a single idea can fit in a single unit of code. Your unit of code can be a function, a class, etc. These are merely tools of organization. As with any tool, it can be over used or used incorrectly. Having a one ...


26

It's incredible to me how many people hate regions so passionately! I agree completely with so many of their objections: Shoving code into a #region to hide it from view is a bad thing. Dividing a class into #regions when it should be refactored into separate classes is clearly a mistake. Using a #region to embed redundant semantic information is, well, ...


25

Multitier Architecture When you have Layers on Layers on Layers on Layers... you see my point here, in your application. I call it Over Layered Architecture Over abstraction in such way that you get lost in the code. Futuristic Architecture This happens when the solution is too futuristic. In reality no one can predict new requirements. Therefore most of ...


24

The example you gave is actually fine in my opinion. You are declaring inner classes, so it is perfectly sensible to keep them in the same file. The only way around this would be to make your Items class a partial class and split it over multiple files. I'd consider this bad practice. My general policy for nested classes is that they should be small and ...


24

I don't think they hate that you add the newlines, but they hate it when you add it to their existing code because it will trigger a change in your SCM, the compare view will show changes even though the new lines semantically aren't doing anything. It's not really a programming mistake, but more like something slightly annoying that could be avoided. In ...


23

Why not have a connection class instead? class Connection(object): def retrieve(self, resource): return something_from_s3() class MockConnection(Connection): def retrieve(self, resource): return 42 def foobar(connection = Connection(), **kwargs): whatever = connection.retrieve("foobar") Not only is this cleaner, but you can ...


23

Those assertions are really useful for testing your assumptions, but they also serve another really important purpose: documentation. Any reader of a public method can read the asserts to quickly determine the pre and post conditions, without having to look at the test suite. For this reason, I recommend you keep those asserts for documentation reasons, ...


22

The code can be simplified like this: boolean condA = ( fooSuccess && !barSuccess && mooSuccess ) boolean condB = ( fooSuccess && !barSuccess && !mooSuccess ) boolean condC = (!fooSuccess && barSuccess && mooSuccess ) boolean condD = (!fooSuccess && barSuccess && !mooSuccess ...


21

I've seen an increasing trend in the programming world saying that it is good practice to separate code blocks into their own functions. I wouldn't have called this an "increasing trend". I was taught that splitting overly large methods into smaller methods improved readability ... ummm ... nearly 40 years ago. And I was taught the design-time ...


18

I just finished reading The Art of Unit Testing a couple of days ago. The author advocates putting as much care into your unit tests as you do your production code. I've experienced poorly-written, unmaintainable tests firsthand. I've written some of my own. It's virtually guaranteed that if a test is a pain in the ass to maintain, it won't be ...


17

Probably because you're not adhering to their style convention. In coding even whitespace is important.


17

Modern OO design accepts that not everything is an object. Some things are behaviors, or formulae, and some of those don't have state. It's good to model these things as pure functions to get the benefit of that design. Java and C# (and others) require you make a util class and jump through that hoop to do it. Annoying, but not the end of the world; and not ...


16

Unfortunately your average developer still thinks of relational databases as big flat files. The only way they will get any better is if someone takes charge and leads by example. Just recently I spearheaded a major redesign of an important schema in our database and followed common relational practices. All of the sudden our stored procedures were more ...


16

The old design saying is, "You can touch your friends, and you can touch your privates. But you can't touch your friends privates." That's coupling in a nutshell. Signs of highly coupled code include very large interfaces that let people know about private details of the implementation, and objects that seem to "know a lot about each other". There are ...


16

Yes, it's OK, and fairly common. It can be non-obvious though, as you've discovered. In general, I tend to have persistence-type methods return the updated instance of the object. That is: Report InsertReport(Report report) { db.Reports.InsertOnSubmit(report); db.SubmitChanges(); return report; } Yes, you're returning the same object ...


15

I don't know about being proud of the fix because it was so obvious, but the most horrible code I remember fixing was this. if (userName=="John O'Reily") { Username = "John O''Reily";} if (userName=="Stacy O'Neil") { UserName = "Stacy O''Neil";} if (userName=="Finnegan O'Connor") { Username = "Finnegan O''Connor";} ... someSQL = "SELECT * from Users where ...


15

Everything is Configurable. When an architect tells you that his system is change-proof or highly-customizable because of extensive configurability, that's an architecture smell. The problem is that you can really only provide configuration mechanisms for what you think right now is going to need to be configured, but once your application is in the wild, ...


15

This really isn't answerable without knowing the exact case you are trying to solve. The reason they call them "code smells" is not because the "smell" itself is bad but rather because it indicates that there is something else wrong, a metaphorical pile of crap underneath generating that smell. As such, the answer generally is not to directly attack the ...



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