What's the reason why table layout in code is considered bad?
My colleagues tell me that table based formatting of code is bad and it is no readable and I should follow conventions. What's that bad about table based formatting? Why is it banned?
I'm asking because for me it's much more readable.
Examples (not real code):
if (res == ResultType.Failure)
something = ProcessFailure(..);
if (res == ResultType.ScheduledAndMonitored)
something = DoSomething(...) && DoSomething3(..);
if (res == ResultType.MoreInfoAvailable)
info = GetInfo(..);
if (res == ResultType.OK && someCondition)
something = DoSomething2(..);
.... continued
versus
if (res == ResultType.Failure) something = ProcessFailure(..);
if (res == ResultType.ScheduledAndMonitored) something = DoSomething(...) && DoSomething3(..);
if (res == ResultType.MoreInfoAvailable) info = GetInfo(..);
if (res == ResultType.OK && someCondition) something = DoSomething2(..);
.... continued
Why I think the second one is better:
- I don't need to parse the text by eyes - I see the structure of the commands at one glance.
- I see immediatelly
- that there are some ifs and assignments
- that the enum used in conditions is ResultType and nothing more
- that only the last condition is composed from two expressions
Update: this is not a real code. I just wanted to show 开发者_StackOverflowsome example. Consider it like it's an old code that somebody wrote some time ago and you need to read it. Why the first formatting is preferred over the second?
When your colleagues say you should follow conventions, does that mean your team has a formatting convention? If that is the case, that is enough reason right there. When everybody formats their code the same way, it is easier to read your teammate's code.
The "table-based" layout you describe can be very useful in certain cases:
public string GetMessageFromErrorCode(int code)
{
switch (code)
{
case 1: return "OK";
case 2: return "Syntax Error";
case 3: return "Other error";
}
}
Note that this example is a valid exception to the c# rule that "All case
statements must also include a break
."
I am not fond of being rigid about code layout. Following a coding standard is a good thing. But as soon as you start doing things like using several parameters with long names in function calls, writing linq queries, or using anonymous methods or intricate lambda statements, it can make your code more readable to break the rules from time to time.
See Also:
Anonymous Methods / Lambda's (Coding Standards)
The reason is simple.
If you are using spaces to indent everything, and make it lineup, it's a pain to keep it inline when a variable or condition changes. You have to manually go in and add/remove spaces.
If you are using tabs to indent everything, and make it lineup, it won't look the same on everyone's computer since not everyone has their tabs set to the same width. Some people have them set to 3 characters, others 5, others something different. And what looks nice and neat and "table like" on your machine, looks like a mess on theirs.
Throw braces around the statements that are executed by the if, and format it normally.
if (res == ResultType.Failure)
{
something = ProcessFailure(..);
}
if (res == ResultType.ScheduledAndMonitored)
{
something = DoSomething(...) && DoSomething3(..);
}
if (res == ResultType.MoreInfoAvailable)
{
info = GetInfo(..);
}
if (res == ResultType.OK && someCondition)
{
something = DoSomething2(..);
}
Oh, and the other reason is, not everyone uses a fixed with font for programming. Believe it or not, some developers use proportional fonts.
The problem that I see with your example is that the eye connects the information vertically, for instance, when I glanced at the code I saw
something
something
info
something
And mentally grouped them, where as in the other, I saw the flow and logic of the code and then the details. This makes it more straight forward to cram what I need to into my head.
I would say there are a few reasons: to reasonably be expected to maintain a layout like this you will need an IDE / plugin that will maintain this for you. Without this support there will be a large ongoing effort that will be required for developers to be spaces and tabs wranglers all day long.
The second and more substantive reason is code that would be benefited by this layout is wrong. Your sample could be written to use polymorphism, the strategy pattern, or other ways that would be significantly better which makes this entirely a moot point.
I think your "table-based" example is far easier to read, on its own. The problem is that we've got 50 years of text-based tools for dealing with source code as lines of text. The two most obvious problems that you'll run into with this are:
All my favorite text-handling tools, like
diff
andgrep
, work on lines. The more you put on one line, the less value my tools have. For example, if I want to grep for something in the if's condition, I have to filter false positives from the ifTrue clause.(If you're writing in a language with more regular syntax in an extensible editor, like Lisp inside Emacs, this isn't as much of a problem, since it's trivial to traverse the actual structures. But C# has very complex syntax, and I don't know of an easy way to traverse my C# code with 1 or 2 lines of code, so text tools still reign here.)
Your longest line is about 95 characters -- and that's with
..
in place of actual args, so it'd be even longer. Really long lines can be a pain to work with. Lines of varying width (since most of my lines won't be that long) mean wasted screen space, since my windows are all rectangular. Or I have to keep resizing windows when I scroll. Any way I look at it, editors and tools are great at handling files with many lines, but pretty lousy at handling lines of wide and varying lengths.
I'd like writing this code. I'd love reading this code. I would really hate having to do a 3-way merge on this code. :-)
In contrast, I'm going to go out on a limb and say I think most of the other answers here so far are bunk:
- various things about char
0x09
: well, use spaces and not tabs, then; that's just common sense these days - monospaced fonts: all kinds of code standards I've seen fall over if you use a variable-width font, not just this
- "it's the convention": OK, but that doesn't say why it's the convention, or why this case shouldn't be an exception to the convention (because every set of code conventions I've ever seen allow exceptions)
- StyleCop says it's bad: are you working for your tools, or are they working for you? see also: "it's the convention"
- it's hard to maintain: not in my editor, and if it's hard in your editor, you need to learn how to use it, or get a better one
- some people won't have your editor: do all your coding standards depend on it being easy to write with an arbitrarily wimpy editor? If we hire somebody who insists on using Notepad, that's their problem, and we're not going to change our source code for them. See also: "it's hard to maintain"
- it's inconsistent: sure, but (since I'm not a robot) that doesn't necessarily make something harder to read; every comic strip in my newspaper is formatted differently (in some cases really differently!), yet I don't find any of them at all difficult to read
(Flame away!)
In the end, I think both of these styles are kind of lousy for this kind of code, for different reasons. If you need a table of stuff, then use switch
, or an IDictionary<ResultType,Action>
, or a list-of-lists or -tuples (if order matters), or offload the rules into a config file, or something. You're right to think that a table should look like a table. The only thing you're missing is that we've got constructs to make things look like tables that would work just fine here, and if
isn't one of them. :-)
I think you have a good case for making an exception to the formatting rules, but my opinion won't help your cause. My true advice is if you are a team and multiple people are telling you to do it one way but you want to do it another, and you stand to lose nothing but pride, it's more productive to move on, gaining a tiny bit of respect as a team player, which might turn into the kind of thing that lets you get your way in the future.
I learned the above by not following it most of my carrear - it is just not worth the headaches sometimes.
I think it depends a lot on the personal choice as well as to what formatting to use. In a professional environment you would expect the team to have a defined and agreed coding standard which all the team members follow.
There are many tools and add ins to IDE's like Visual Studio which give you the capability to suggest changes to the code structure by parsing the source code files.
One such too which i find very useful is the [StyleCop][1].
I personally find Stylecop very useful to have a consistant coding guideline.
Regards, Nilesh
If you feel the need to tabularize your code, it's the right time to think. It smells like refactoring time. Code should be readable like a sentense.
Second reason is the code maintability cost. Tabularized code is harder to maintain.
The key to readability is consistency. If the same construct appears in different ways at different times, then it will be more difficult to process mentally. Therefore, I would not like to maintain C# code that was formatted in a tabular style.
Also, tabs lie.
if (!HydrogenTankFull()) FillHydrogenTank();
if (!OxygenTankFull()) FillOxygenTank();
if (HydrogenTankFull() && OxygenTankFull()) TestGimbals();
if (GimbalsTested() && LaunchButtonPressed()) IgniteEngine(); BlowRestrainingBolts();
if (LaunchPadCleared()) TransferControlToHouston();
Why did the rocket just fall over?!
Hard to say what is going on in your sample, but it looks like you are doing a lot. If so, it to me it seems wrong that you want to cram so much logic into just 4 lines. This is C#, not Python. Speaking of C# vs Python, you are not using braces for if statement body, which MANY would not like. StyleCop would not like it either. Speaking of StyleCop, you also might want to run FxCop as well as ReSharper - it might suggest a cool way to improve the code. Why are you not using return
? Even if the conditions are mutually exclusive, I find return
very useful - allows to skip the rest of stuff without using lots of else if
, yet still be able to detect a so called "default" case (think switch
). My stab at it:
// This should be in it's own function; I am just too lazy to spell out a signature.
{
if (res == ResultType.Failure)
{
return ProcessFailure(..);
}
if (res == ResultType.ScheduledAndMonitored)
{
bool result = DoSomething(...);
result = result && DoSomething3(..);
return result;
}
if (res == ResultType.MoreInfoAvailable)
{
info = GetInfo(..);
// Ok, and then?
}
if (res == ResultType.OK && someCondition)
{
return DoSomething2(..);
}
// Else ...
Debug.Assert(false, "Why did we get here? Explanation: ");
}
What I wrote above is not the best code, but I tried to clean it up. Do not worry about extra variables - the optimizing compiler will take care of them. Do go for logical readability. In my strong opinion, putting things into tabular format does not help. If code is too hard to read once StyleCop is happy, then you probably need to re-factor it, as well as simplify logic/state/etc.
Without know exactly what you are trying to do, it is hard to give a better code sample.
Focusing in on the example given, I would format it like this. I find both your examples hard to read.
if (res == ResultType.Failure)
{
something = ProcessFailure(..);
}
if (res == ResultType.ScheduledAndMonitored)
{
something = DoSomething(...) && DoSomething3(..); //Bit-and on the results? huh.
}
if (res == ResultType.MoreInfoAvailable)
{
info = GetInfo(..);
}
if (res == ResultType.OK &&
someCondition)
{
something = DoSomething2(..);
}
But, supposing this was an actual snippet, I would have written it as a switch/case sort of thing. Possibly with careful use of dropping down from the previous break. :o
Even though I also find your second (table-like) example more readable, I would not use it because
- This kind of layout is not easy to maintain (you'll possibly have to go through the whole list if one line changes). Therefore it's highly probable that someone will eventually be too lazy to adjust it and the layout will become an unreadable mess.
- If a single line has, say, three or four conditions, all lines will become unnecessarilly long and therefore unreadable.
- And of course, if it's against the coding standard in your team, please just don't do it :) ...
It would be nice if programming languages, markup languages, and text editors could all cooperate with each other to allow text formatting to adapt itself sensibly to fit the code. For example, if a comment associated with a line of code would be too long to fit to the right of that line, and if succeeding lines of code are shorter, allow the comment to wrap and remain to the right of those succeeding lines of code, with some sort of arrow or box to show that the continuation lines of the comment all belong to the first line of code.
Unfortunately, I know of no widespread programming languages which fit well with any means of embedding markup.
In otherwise top-down code flow, your brain has to switch to the "table reading mode" for this code block. And make sense of what the table "columns" are. And recognize the end of the table block and switch to the normal reading mode.
Too much hassle IMO.
It's not more readable to all of us. Imagine if regular English text had strange tabulations in it to line up the subjects and predicates of sentences. That'd be hard for the eye to follow, no? I read code much like I read prose, so when I have to jump around horizontally a lot, it's a strain to me. However, using tabs when initializing data structures is helpful since that's more-logically a table to me.
So that's why code formatting standards are important; there's no one true way to best format code that will please everyone.
Consider the case of parameter validation and possible early exit in a function. Sometimes I like to format my code similar to the original example to reduce the validation code in relation to the rest of the code. I could be validating the input prameters or I could be checking the validity of hte requested operation based on the internal state of the object. For example:
void LogMessage(LogLevel lvl, string msg)
{
if (!this.loggingManager.IsLogging) return;
if (lvl < this.loggingLevel) return;
this.loggingManager.Write(this.loggerName, msg);
}
Or
void DrawGeometry(Graphics g, Geometry geom, Layer layer, double scale)
{
if (geom == null) return;
if (!layer.IsDisplayed) return;
if (this.isDisplayByScale && (scale < this.minScale || scale > this.maxScale) return;
if (this.ColorsToTreatAsTransparent.Contains(layer.Color)) return;
Geometry xformed = this.Transform(geom);
using (Pen p = new Pen(layer.Color, layer.Width))
{
g.Draw(xformed, p);
}
}
Note that both of these are completely made up examples. I go back and forth, but I do kind of like the brevity of the one line sanity check/return statements.
Curly braces to create statement blocks are just more common industry wide, build the habit now because most people will think your code looks dinky if you write it like that.
On another note, one of the reasons I've seen the rule held that you always use curly braces for if statements and never do an if (condition) single-line-statement; is because it's not uncommon for a line of code to change 4 months later causing the if statement to fall through to the next statement without you realizing it, therefore as safety, if's should always be:
if (condition)
{
single-line-statement;
}
精彩评论