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 ...
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 ...
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.
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 ...
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
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 ...
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
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 ...
2
I think you need to ask why the field was set to final in the first place. The impression is that it wasn't ever intended to be changed.
The fact that you need to change the value (or have multiple options for the value) indicates that the original assumptions driving the design are no longer valid.
I would recommend thinking about what function that ...
2
If I were you, I'd create a function:
template< typename PodType >
whatever buffer(PodType& obj) { return buffer(&obj, sizeof(obj)) }
But this is so obvious an omission in the set of overloads provided, that I wonder if I'm missing something.
2
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
...
2
Looking at the OP's initial approach, it is immediately obvious to me what it's doing. I don't see that there are readability or maintainability problems with this at all, nor do I see that it violates any commonly-accepted Python conventions or idioms. It's clear, and it uses minimal syntactic boilerplate and extraneous operations to achieve its goal. I ...
2
You should be consistent, but not necessarily with the old code. That is, your team should agree on the right way of doing something, and use that way whenever new code is written or substantial changes are made.
That way, you reap most of the benefits of consistency (in particular, it won't matter who writes the code), but can still improve if the team ...
1
In my opinion what you are saying - all static functions to be nullipotent - is a good OO way of writing code in most cases but I do not believe that when you look at a static function you should automatically assume that it does not have side effects. The situation may be more complex; take for example the function Class.forName(String) which appears to be ...
1
That being said, what clues should I have in my code for someone coming along to maintain it to know what's stateful and what's not?
You can make them static. FxCop over in the C# world will push you to make things static that do no reference member variables. This is the correct thing to do (usually).
When I realized what a pain it was to mock ...
1
[snip due to people not reading the /entire/ response]
To answer your actual question, whitespace is fine to break things up visually, but I'd recommend that the number of lines that you use should not exceed that of the number of lines between other entities in your code (such as number of lines between functions within a class, or module-level classes or ...
1
You can use void * (or IUnknown *) to achieve loose coupling and reduce code complexity. It eliminates circular referencing in C++ header files.
// Model.h
class CModel
{
public:
void addBlock(char *str, void** ppBlock);
};
// Block.h
class CBlock
{
public:
void getParent(void** ppModel);
};
Then with appropriate casting, you can get what you ...
1
For "pure" ASP.NET, there isn't really much of an issue because by design, the aspx page and the code-behind document are separated: the code-behind document follows the indentation rules of the language used (C#, VB.NET, ...), the aspx page follows HTML markup.
If your ASP.NET pages contain e.g. JavaScript-like scripts, then I try to minimize their ...
1
I'd modify the Customer class to have a getAvailableCredit() method.
Ideally, I'd like to see this as the "Working" line of code:
if (order.getTotal() <= customer.getAvailableCredit() ) order.Ship();
In particular, I'd like to see a PurchaseOrder class that would encapsulate the the set of items, which would total itself, and add an relevant taxes, ...
1
IndentGuide ( http://atlanto.github.com/IndentGuide/ ) does the trick. Select Help->Install Software and add the URL to "work with" http://atlanto.github.com/IndentGuide/update
Only top voted, non community-wiki answers of a minimum length are eligible
