Is this a coding horror?
-
This is what I was thinking as well, which is why I looked at all the responses. The only I would change, in .NET 4, is to use the HasFlags method of enum:
var rolesList = user.Roles.HasFlag(userRole) ? inRoles : outRoles;
rolesList.Add(roleName);This also means no brackets are necessary.
-
Up to a point. Code which contains nothing but several levels of 'well named functions' wrapped around a simple arithmetic calculation are a pain to debug (trying to actually find the places where calculations are done and therefore being able to see whether they're done right). It's kind of like trying to read a book by just looking at the contents page. It's hard to read such long names, too (unless you put in underscores to make it look like a sentence but then the arguments seem to be bound to the last word). This would go in a function for me if I were using it in more than one place (which in this example I almost certainly would be). Your proposed name misses the most important point, by the way. It should be called "STPVolume" or "GetVolumeAtSTP" or something like that.
You think debugging is more difficult with more functions? I disagree. My IDE steps into functions, or over them...which makes debugging simpler, not harder. I can step over functions I've already eliminated as the problem, and into those which could be an issue. Rather than stepping on every line of code. And I should know if the bug is in a function or not by writing tests for it. My "STPVolume" or "GetVolumeAtSTP" function should have test methods ensuring it's accurate. As for the name, physics isn't my domain, so I didn't know the proper name, I made my best guess. The essence of my point still stands.
-
Definitely agree with you. But at work, I'm always surprised to see the number of junior developers who don't understand (or don't want to try to understand) this simple construct.. (the "?" operator, I mean)
-
You think debugging is more difficult with more functions? I disagree. My IDE steps into functions, or over them...which makes debugging simpler, not harder. I can step over functions I've already eliminated as the problem, and into those which could be an issue. Rather than stepping on every line of code. And I should know if the bug is in a function or not by writing tests for it. My "STPVolume" or "GetVolumeAtSTP" function should have test methods ensuring it's accurate. As for the name, physics isn't my domain, so I didn't know the proper name, I made my best guess. The essence of my point still stands.
Yes. IDE stepping is the last resort of debugging, one should be able to find the suspect piece of code quickly without diving through a big tree of methods. (I'm suffering from this at the moment in a real project, in fact.) Obviously, you don't just want the whole application in one big main(), but taking something which is only one or two lines out of line does not (imo) help readability. (Taking it out to avoid duplicating the calculation is a separate matter and a good reason to do so, because the pain of having duplicate code massively outweighs the problem of having too many functions.) Your tests will tell you if the function is 'accurate', but it won't tell you some things which may be important when you want to use it (that you didn't think of at the time). For example, just what is standard temperature and pressure? ... there are different definitions and to find which one is being used you need to find the calculation. This is more common with more complex methods, or order of event firing or other slightly tricky things like that.
-
Yes. IDE stepping is the last resort of debugging, one should be able to find the suspect piece of code quickly without diving through a big tree of methods. (I'm suffering from this at the moment in a real project, in fact.) Obviously, you don't just want the whole application in one big main(), but taking something which is only one or two lines out of line does not (imo) help readability. (Taking it out to avoid duplicating the calculation is a separate matter and a good reason to do so, because the pain of having duplicate code massively outweighs the problem of having too many functions.) Your tests will tell you if the function is 'accurate', but it won't tell you some things which may be important when you want to use it (that you didn't think of at the time). For example, just what is standard temperature and pressure? ... there are different definitions and to find which one is being used you need to find the calculation. This is more common with more complex methods, or order of event firing or other slightly tricky things like that.
-
I've never heard of "the problem of having too many functions"... Have you read books like Clean Code or Code Complete, etc?
-
I don't know - it isn't in my code anymore... I wrote this wonder this morning, and realized I could prune it down to just the one line:
private void SetRole(SMUser user, UserRole userRole, string roleName, List<string> inRoles, List<string> outRoles)
{
(((user.Roles & userRole) != 0) ? inRoles : outRoles).Add(roleName);
}I mean, yes it works. But... It's not there anymore: I changed the whole way I handle roles for other reasons. But still, It's a bit impenetrable...
Real men don't use instructions. They are only the manufacturers opinion on how to put the thing together. Manfred R. Bihy: "Looks as if OP is learning resistant."
It looks perfectly simple to me. I'd be very disappointed if a programmer couldn't understand it!
-
I don't know - it isn't in my code anymore... I wrote this wonder this morning, and realized I could prune it down to just the one line:
private void SetRole(SMUser user, UserRole userRole, string roleName, List<string> inRoles, List<string> outRoles)
{
(((user.Roles & userRole) != 0) ? inRoles : outRoles).Add(roleName);
}I mean, yes it works. But... It's not there anymore: I changed the whole way I handle roles for other reasons. But still, It's a bit impenetrable...
Real men don't use instructions. They are only the manufacturers opinion on how to put the thing together. Manfred R. Bihy: "Looks as if OP is learning resistant."
hmm... The only thing I'd argue with is doing all that then adding the adding the "Add" method to the end. Being used to the ternary operator, I could read it fairly quickly, but to make it more human readable, perhaps an assignment a variable should have been assigned to the chosen list, then the second line should have implemented the Add method. In ANSI C in college, they encouraged this type of stacking behavior. In fact, they demanded it, but it was more of a performance issue than simply being "clever."
-
No, not always, there are some rare cases where cluttering the code with comments are justified. But if you look at some code and thinks "gosh I better comment it, so other people understands it" then it is an indication that the code is a horror, why do you need to explain the system whit a language that is not designed to describe a system, when you allready have described it with a language that is designed to excplain a system? Wouldn't it be better to left click, refactor->rename and clean up the code so it tells the story you want it to tell, insted of trying to excplain the story by including a secondary language into your source files? also comments tends to become misleading and/or misplaced as the system evolves, so instead of excplaining the system it just adds to the confusion. so, hyperboled, I say, a well commented code is a indication that the system is dirty. and commenting in itself can also make clean code dirty by breaking up the logic and lie to you while you read the code. In most cases you could explain what the sysem is supposed to do better whit a programming language than whit english or swahili. And if you, god forbid, uses comments to add todos, changelogs or similar there are plenty of good tools out there for that.
@annathor I mean no disrespect but you are %100 wrong about good code not needing comments. It is this kind of mindset, that everyone thinks exactly alike, that leads to so many re-writes and problems. I used to work for a software company that employed many developers who had this same approach to commenting anything development related and it lead to numerous hours of unnecessary support simply because the developer assumed that the way they saw some scenario was the only way and therefore the same way all others would see it. This doesn't mean that ALL code must be commented for there are some simple ones that are fairly self-explanatory but never assume code can be left uncommented if its good code.
-
While I mostly agree, this is not always possible. C++ in particular is such a large language that few people know it completely, but its perfectly possible to write excellent programs using a familiar subset. In such cases, use the arcane features of the language can be regarded as a codin horror - as the majority of developers will need to look up the meaning. While programmers favouring other languages may scoff, this seems similar to knowing the libraries. I could learn C# syntax in at most a week for example, becoming familiar with the libraries takes longer, and is actually what I spend of my time doing. I'm absolutely sure I don't know them as well as I'd like, and there's whole areas I leave until I need to. What makes all this work is MS's documentation that (normally) helps see usage conveniently. Most of that is auto-generated from Comments, so comments seem fairly necessary. I kind of hate the "everyone who doesn't know what I know is an idiot" approach to programming - I'd hate to be on a project with someone with that attitude, they tend to leave unmaintainable code in their wake.
