Hot answers tagged code-style
38
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 using the best possible method to accomplish your task. There is no one correct answer to this dilemma; it depends on:
How beneficial is the "correct" way of doing things? On one end of ...
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 ...
18
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){
...
15
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. Oh yeah and well we don't have time for this do we?
I'd rather have ...
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.
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 ...
9
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 ...
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 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 ...
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 ...
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
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 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
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
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
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
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.
...
3
I split up function even if I don't plan to reuse the functionality.
Also I try to keep function size smaller than the screen-size.
This brings 2 advantages, first I can grasp what the function does without scrolling and I can minimize the comments, because every function gets a speaking name.
3
I do prefer the second example. My reasoning is that even if this is the only place in the code you need to check the user's credit right now, does not mean that this will always be the case. Now that you have abstracted out this code it will be more reusable if you ever run into a situation where you do need it.
However, like anything else this can be ...
3
Theres an important detail missing from the equation. Why does tag contain post and visa-versa? The answer to this question determines the solution for each given set.
In general I can think of a situation that is similar. A box and contents. A box has contents so a has-a relationship is appropriate. Can contents have a box? Sure, a box with a box. ...
3
There is no better choice in general. Code readability and system performance are two valid goals, and if they are at odds, you must know about the strengths of those two forces in your particular situation before deciding. To say, "when readability and performance are at odds, always choose performance (or readibility)" would be a terrible ...
3
Quick googling shows that some research has been done. For instance, this paper shows that there's a value of cyclomatic complexity of code that minimizes bug rate:
Probably deep nesting may be fine as long as it does not branch at every point. That is, a having many nested conditions on top, as in your example, is probably fine, since it's essentially ...
2
In addition to what @AGC said, if the doSomething() belongs to say, a customer class, then you don't want to make that static. In other words, business classes are seldom made into static classes since their data is supposed to change in the scope of a single transaction and you don't want to keep them around all the time.
Static type classes are good for ...
2
I think comments should refer to the code that's there, not to code that has been there in the past, nor to events that happened to the code sometime in the past, nor to code that existed in a parallel universe (another branch) in the past. Leaving the markers in the way your team member did creates at least three problems:
The original code probably was ...
2
The "type" of types shouldn't matter. class, struct, and enum are only meaninful to the definition - you shouldn't be using something differently just because it's a struct and not a class, nor should you refer to it as one.
I think the most sensible way to use "hungarian notation" is to only seperate types from functions:
types
instances of types
...
2
While in theory things like that can go either way, in practice when you get down to the implementation one way is almost always a better fit than the other. My hunch is it will fit better in the Post class because the association will be created during post creation or editing, when other things about the post are changing at the same time.
Also, if you ...
Only top voted, non community-wiki answers of a minimum length are eligible
