Tag Info

Hot answers tagged

47

To give a more general answer: In a case like this, you have two programming "best practices" that are opposed to each other: code consistency is important, but so is choosing the best possible method to accomplish your task. There is no one correct answer to this dilemma; it depends on a couple factors: How beneficial is the "correct" way? Sometimes ...


33

The apparent need for a go-to statement arises from you choosing poor conditional expressions for the loops. You state that you wanted the outer loop to continue as long i < 10 and the innermost one to continue as long as j > 0. But in reality that's not what you wanted, you simply didn't tell the loops the real condition you wanted them to evaluate, ...


32

In this instance, you could refactor the code into a separate routine, and then just return from the inner loop. That would negate the need to break out of the outer loop: bool isBroken() { for (i = 0; i < 10; i++) for (j = 10; j > 0; j--) if (j == i) return true; return false; }


25

This is clearly wrong. It is the job of the version control system to keep track of changes, and it is the job of diff tools to show what has changed as a result of the merge. There should be a comment in the commit log, and maybe in the code, explaining what was changed and why. However, IMHO, leaving the conflict markers in as comments is the same as ...


20

Staying consistent has little value in my perspective; continuously making improvements is a must. Your colleague's position really impedes innovation. The consistency argument gets you into a situation where you can use, for example, LINQ only if you migrate all code to use LINQ. And well, we don't have time for this, do we? I'd rather have inconsistency ...


19

Here is what Robert C. Martin (the "clean code" guy) would do: http://blog.objectmentor.com/articles/2009/09/11/one-thing-extract-till-you-drop So I guess he would split up your isCreditOk function further like this public int calculateOrderPrice(Set<Item> items) { int orderPrice=0; for(Item item: items){ ...


18

The refactored example is objectively superior because it has less repeated code. Therefore, that implementation is actually simpler, and probably shorter. There is also the argument that the original code in your example is an ad-hoc implementation of traits (or an awkward expression of the strategy pattern), the result being unnecessarily complex code. In ...


11

Joel wrote a good article on this Making Wrong Code Look Wrong In short, you want to use Apps Hungarian, not Systems Hungarian notation. Because really, who cares if it's an int, or a long, or whatever, all I care about is the "kind" of value, not it's type.


10

Your second example is more readable (although I would invert if to reduce nesting and rename isCreditOk to isPurchaseAllowed, and possibly moved the latter to Customer). First one hides code intent, exposing implementation, thus while it's clear what code exactly, it's not clear why it does those things.


10

Like the Pirate Code, the SRP is more of a guideline than a rule, and it's not even a particularly well-worded one. Most developers have accepted the redefinitions of Martin Fowler (in Refactoring) and Robert Martin (in Clean Code), suggesting that a class should only have one reason to change (as opposed to one responsibility). It's a good, solid (excuse ...


9

I tend to think less in "paradigms" and more in terms of using the most appropriate (i.e. - simple, readable and maintainable) tool for the job. I don't believe in sticking rigidly to paradigms because I feel that often results in going out of your way to write code in a certain manner just for the sake of adhereing to the "rules". However it's certainly ...


9

I'd figure out a new variable name instead of trying to figure out what naming convention to use for a double-number variable That said, if I absolutely HAD to use a double-number in my variable name, and I couldn't rearrange the words/numbers to come up with something reasonable, and absolutely no other name would make sense, I would use an _ between the ...


8

Creating an object only to call some method on it and then discard after the call is generally a bad idea. So, one possible solution is to convert that instance method to a static method. You can also leave it as an instance method but use only one instance of that class throughout the code. The latter can be enforced using the Singleton Pattern, as @AJC ...


8

I agree with Kevin Cline. You ought to extract those four lines in a method. It's true that the number of lines of code you save is small, but the process gets much more straightforward, you have less clutter, more testability, and less risk of forgetting one line. The "Don't Repeat Yourself" principle has less to do with saving lines of code (sometimes, ...


8

This is something that you need to take on a case by case basis. I would argue that you should use unfamiliar features of a language if it would improve clarity and make future maintenance easier (both of which your example accomplishes in my opinion). You should never use features just because you want to be clever or because you want to use the feature ...


7

I liked the old way because the capital letter told me that, just like Math.sin(x), ThirdPartyLibraryWrapper.newThingy(x) did the same thing the same way every time. There's no object state to change how the object does what I'm asking it to do. That is the right way to do it, yes. That said, static confers no guarantees of immutability or state ...


6

API consistency is very important, both for public and internal APIs. Code formatting consistency is important, and should ideally be enforced by automatic formatting tool with same formatting rules for everybody. Makes living with shared version-controlled codebase easier. Naming conventions should be consistent, also for things like local variables etc. ...


6

No, I don't think a goto is necessary. If you write it like this: bool broken = false; saved_i = 0; for (i = 0; i < 10 && !broken; i++) { broken = false; for (j = 10; j > 0 && !broken; j--) { if (j == i) { broken = true; saved_i = i; } } } Having &&!broken on ...


5

This decision hinges on two (possibly contradictory) principles/guidelies: Single Responsibility Principle and You ain't gonna need it. Per SRP you should be thinking about class boundaries, and not method boundaries here, but for the sake of this question, let's just talk about methods. A huge benefit you get out of SRP is that each of your classes/methods ...


5

I've been throw a very similar situation when I had to deal with a terrible legacy Windows Forms code written by developers that clearly didn't know what they were doing. First of all, you're not overacting. This is bad code. Like you said, the catch block should be about aborting and preparing to stop, it's not time to create objects (specially Panels). I ...


5

No, not in both! It should be in one place. What I find uneasing in your question is the fact that you say "Post takes care of creating posts, deleting posts, updating posts" and same for Tag. Well, that is not right. Post can only take care of updating, the same for Tag. Creating and deleting is the work of someone else, external to Post and Tag (let's ...


5

I've had similar problem with some code either being commented out (which is somehow simillar to your case) or moved to a method not being actually called anywhere. When asked why people do this the response was that they feel a bit safer when they have some code block still around. The most obvious counter argument is that it is VCS job and not theirs. ...


5

This depends offcourse in what exactly you are doing in that function and If you can call it as static or not. (static variables inside, etc...) But from the way you seem to use it, the answer is yes. Either declare a static class or use the Singleton Pattern. But don't keep doing what you are doing. You are just having the overhead off instantiating a new ...


4

Conceptually, you're already calling a static function, so the easy answer is to just go ahead and code them that way. A harder answer is that you probably have an underlying design flaw, such that this object is either a "misc" that you just threw a bunch of left over code into, or you are passing it a lot of state on each of those calls. While they are ...


4

Divide into smaller functions is the best way for sure, at least for human readability. By extracting a smaller function, you are naming a block of code. That really improves readability. Readers can have a macro idea of what that function does, without having to bother with details of each step. If you want to see the details, than you look ate the ...


4

do you also tend to select a preferred programming paradigm when coding and stick to it, or do you tend to explore more and mix different paradigms? Constantly exploring alternative solutions makes you a better programmer. You must be aware of available tools, it doesn't mean that you have to use them each time. Mixing paradigms is fine as long as ...


4

I think splitting up functions is a good idea. But you need to be careful. If you simply split functions for splitting's sake you end up with a bit of a mess. When I've seen Uncle Bob's stuff he often seems to be splitting without thought. Decomposing a function requires care and skill, and shouldn't be done lightly. public boolean isCreditOk(Customer ...


4

Unfamiliar with LINQ? If so, wouldn't my code be more maintainable for my fellow developers if I didn't use it? The C# language is still evolving. If people didn't learn the changes from C# 1, they would be missing out on: Generics Partials Anonymous methods Iterators Nullable types Auto-properties Anonymous types Extension methods Ling Lambdas ...


4

I think your intention in making all local variables final is very good practice, but doing so implicitly (i.e. not explicitly with the final keyword) is a bad idea. Rather than worry about readability for yourself, think about future maintainers of your code: will anyone reasonably expect that reassigning a field without the final keyword will fail to ...


4

The conventions I use are //non-private variable variableName //private variable _variableName //methods MethodName //parameters parameterName //Properties PropertyName These are standard MS c# conventions. There is a list here. If you want something to help you keep nice clean code following conventions, consider Resharper add-on for Visual Studio



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