代码质量 [英] Code quality
问题描述
嗨！
我已经把这段代码写成了我的程序的一部分而且我想知道你是否会介意看看
try_subtract_region并告诉我它是否写得好。
感谢您的帮助！
struct region {
int left，right ;
int顶部，底部;
};
/ *如果两个区域相交，则返回1 * /
static int intersect（struct region * r1，struct region * r2）
{
return（r2-> right> r1-> left&&& r2->> r1-> top&&
r1-> right> r2-> left&& r1-> bottom > r2-> top）;
}
/ *如果r1覆盖整个r2 regin，则返回1 * /
static int covering（struct region * r1，struct region * r2）
{
return（r1-> left< = r2-> left& & r1-> right> = r2-> right&&
r1-> top< = r2-> top&& r1-> bottom> ; = r 2->底部）;
}
/ *尝试从r1中减去r2。只减去生成的区域
*是否为矩形，否则不执行任何操作。
*成功减法时返回1，如果不执行任何操作则返回0。 * /
static int try_subtract_region（struct region * r1，struct region * r2）
{
/ *如果区域不相交其他，然后
*我们无事可做。 * /
如果（！intersect（r1，r2））返回0;
/ *如果r2覆盖整个区域，则相同。 * /
如果（覆盖（r2，r1））返回0;
/ *由于区域2未覆盖整个区域1，我们可以制作
*某些假设，也就是说，如果区域2更靠右边，左边
*并且低于区域1，它不会覆盖顶部，我们
*删除区域1的底部。* /
/ * r2更宽* /
if（r2- > left< = r1-> left&& r2-> right> = r1-> right）{
if（r2-> top< = r1- > top）{/ * r2覆盖顶部* /
r1-> top = r2->底部;
返回1;
}
否则if（r2-> bottom> = r1-> bottom）{/ * r2覆盖
bootom * /
r1-> bottom = r2-> top;
返回1;
}
}
/ * r2更高* /
如果（r2-> top< = r1-> top&& r2-> bottom> = r1- >底部）{
if（r2-> left< = r1-> left）{/ * r2覆盖了左边部分* /
r1-> left = r2->右边;
返回1;
}
else if（r2-> right> = r1-> right）{/ * r2覆盖右边
部分* /
r1-> right = r2-> right;
返回1;
}
}
/ *如果我们到了这里，我们什么都不能做* /
返回0;
}
Edward Gregor写道：我已经把这段代码写成了我的程序的一部分而且我想知道你是否会介意看看
try_subtract_region并告诉我它是否写得好。
编程风格是一个非常主观的问题，评论是绑定
来产生火焰，但这里有一些想法。
struct region {
int left，right;
int top，bottom;
};
添加评论，说明此结构代表什么。是限制
包含还是排他？
/ *如果两个区域相交，则返回1 * /
static int intersect（struct region * r1，struct region） * r2）
分成两行（整个程序相同）：
static int
intersect（struct region * r1，struct region * r2）<返回（r2->右> r1->左&& r2->> r1-> top&&
r1-> right> r2-> left&& r1-> bottom> r2-> top）;
}
/ *如果r1是，则返回1覆盖整个r2 regin * /
静态int覆盖（struct region * r1，struct region * r2）
返回（r1-> left< = r2-> left& ;& r1-> right> = r2-> right&&
r1-> top< = r2-> top&& r1-> bottom> = r2 - >底部）;
}
/ /尝试从r1中减去r2。仅减去生成的区域
*是否为矩形，否则不执行任何操作。
*成功减法时返回1，如果不执行任何操作则返回0。 * /
格式块注释为：
/ *
*尝试从r1中减去r2。只减去生成的区域
*是否为矩形，否则不执行任何操作。
*成功减法时返回1，如果没有完成则返回0。
* /
static int try_subtract_region（struct region * r1，struct region * r2）
{
/ *如果这些区域没有相互交叉，那么<我们无事可做。 * /
if（！intersect（r1，r2））返回0;
/ *如果r2覆盖整个区域，则相同。 * /
if（覆盖（r2，r1））返回0;
我的观点是上述两条评论过多，但这是主观的，取决于谁会阅读你的代码。 OS内核黑客
会将它们视为噪音;在他们向你展示的作业项目中
了解你在做什么。
/ *由于2区不覆盖整个地区1，我们可以制作
*某些假设，也就是说，如果区域2更靠右边，
剩下*并且低于区域1，它不会覆盖顶部，我们
*移除区域1的底部。 * /
/ * r2更宽* /
if（r2-> left< = r1-> left&& r2-> right> = r1- >右）{
if（r2-> top< = r1-> top）{/ * r2覆盖顶部* /
r1-> top = r2->底部;
返回1;
}
上面的两条评论不一致：一个在if之前，一个在
之后如果。
另外，在您使用的样式中，如果
，则应格式化级联....否则如果序列为：
}否则如果（...）{
-
Diomidis Spinellis
代码质量：开源视角（Addison-Wesley 2006）
http://www.spinellis.gr/codequality?clc
Diomidis Spinellis写道：Edward Gregor写道：< blockquote class =post_quotes>我已经把这段代码写成了我程序的一部分，我想知道你是否会介意看看
try_subtract_region并告诉我它是否写得好。
编程风格是一个非常主观的问题，评论必然会产生火焰，但这里有一些想法。
>结构区域{
> int left，right;
> int top，bottom;
> };
添加评论，说明此结构代表什么。是限制还是包含限制？
对不起，您的意思是包容性和排他性限制？
感谢您的帮助！
Diomidis Spinellis< dd*@aueb.gr>写道：Edward Gregor写道：我已经把这段代码写成了我程序的一部分，我想知道你是否会介意看
try_subtract_region并告诉我它是否写得很好。
编程风格是一个非常主观的问题，并且评论必然会产生火焰，但这里有一些想法。
[snip]
> / *如果两个区域相交，则返回1 * /
> static int intersect（struct region * r1，struct region * r2）
分成两行（整个程序中相同）：
static int
intersect（struct region * r1 ，struct region * r2）
正如你所说，这是一个非常主观的风格问题;在这个
特定点上我的风格与你的不同。
我更喜欢将函数类型和名称放在一行上。我看到很多
的代码将函数名称放在一行的开头;我认为*这是因为有些工具可以更好地使用
布局。你可以说这个函数的名字是最重要的信息，并且把它放在行的开头
就能让它脱颖而出。这是一个有效的观点（嗯，这是一个稻草人争论的对立面是什么？）但是我发现这是人为的，而且b / b是笨拙的。 br />
我注意到K& R和标准都不这样做。
当然，如果你的组织需要这种风格，或者你是' '
处理已使用它的代码，你应该遵循它。
[snip]
此外，在你正在使用的风格，你应该格式化级联
如果...其他如果序列为：
}否则if（...）{
再次，我自己的风格不同。在我喜欢的风格中，一个开口支撑
在一行的末尾（除了一个函数的外支撑
定义），并且一个结束支撑在一行的开头*由
本身*。例如：
if（condition1）{
/ * ... * /
}
else if（condition2）{
/ * ... * /
}
else {
/ * ... * /
}
我不确定是不是有理由不把其他放在或
否则如果在与}相同的行上，除了它是我以前用的b
。两种风格都很常见，两者都是内部一致的。
另一个风格点：我总是在条件语句上使用大括号，即使只有'b $ b'一份声明受到控制。例如，我
写这个：
if（condition）{
statement;
}
从来没有这个：
如果（条件）
声明;
唯一一次我可能违反这个规则的是，如果我把整个东西放在
一行，我很少这样做：
if（条件）陈述;
（这是我从Perl那里学到的习惯，需要大括号，但是
我发现C对它来说是一个好主意好吧。）
-
Keith Thompson（The_Other_Keith） ks ** *@mib.org < http://www.ghoti.net/~kst>
圣地亚哥超级计算机中心< *> < http://users.sdsc.edu/~kst>
我们必须做点什么。这是事情。因此，我们必须这样做。
Hi!
I''ve written this code as a part of my program and I
wonder if you would mind looking at the
try_subtract_region and tell me if it well written.
Thankful for help!
struct region {
int left, right;
int top, bottom;
};
/* Return 1 if the two regions are intersecting */
static int intersect(struct region *r1, struct region *r2)
{
return (r2->right > r1->left && r2->bottom > r1->top &&
r1->right > r2->left && r1->bottom > r2->top);
}
/* Return 1 if r1 is covering the whole r2 regin */
static int covering(struct region *r1, struct region *r2)
{
return (r1->left <= r2->left && r1->right >= r2->right &&
r1->top <= r2->top && r1->bottom >= r2->bottom);
}
/* Try to subtract r2 from r1. Only subtract if the resulting region
* is a rectangle, otherwise, do nothing.
* Returns 1 on successful subraction, and 0 if nothing is done. */
static int try_subtract_region(struct region *r1, struct region *r2)
{
/* If the regions are not intersecting each other, then
* we have nothing to do. */
if (!intersect(r1, r2)) return 0;
/* Same goes if r2 is covering the whole area. */
if (covering(r2, r1)) return 0;
/* Since region 2 is not covering the whole region 1, we can make
* certain assumtions, that is, if region 2 is more to the right, left
* and bottom than region 1, it won''t cover the top and we
* remove the bottom part of region 1. */
/* r2 is wider */
if (r2->left <= r1->left && r2->right >= r1->right) {
if (r2->top <= r1->top) { /* r2 is covering the top part */
r1->top = r2->bottom;
return 1;
}
else if (r2->bottom >= r1->bottom) { /* r2 is covering the
bootom */
r1->bottom = r2->top;
return 1;
}
}
/* r2 is taller */
else if (r2->top <= r1->top && r2->bottom >= r1->bottom) {
if (r2->left <= r1->left) { /* r2 is covering the left part */
r1->left = r2->right;
return 1;
}
else if (r2->right >= r1->right) { /* r2 is covering the right
part */
r1->right = r2->right;
return 1;
}
}
/* If we got here, we couldn''t do nothing */
return 0;
}
Edward Gregor wrote:I''ve written this code as a part of my program and I
wonder if you would mind looking at the
try_subtract_region and tell me if it well written.
Programming style is a highly subjective issue, and comments are bound
to generate flames, but here are some ideas.
struct region {
int left, right;
int top, bottom;
}; Add a comment saying what this structure represents. Are the limits
inclusive or exclusive?
/* Return 1 if the two regions are intersecting */
static int intersect(struct region *r1, struct region *r2) Split into two lines (the same throughout the program):
static int
intersect(struct region *r1, struct region *r2)
{
return (r2->right > r1->left && r2->bottom > r1->top &&
r1->right > r2->left && r1->bottom > r2->top);
}
/* Return 1 if r1 is covering the whole r2 regin */
static int covering(struct region *r1, struct region *r2)
{
return (r1->left <= r2->left && r1->right >= r2->right &&
r1->top <= r2->top && r1->bottom >= r2->bottom);
}
/* Try to subtract r2 from r1. Only subtract if the resulting region
* is a rectangle, otherwise, do nothing.
* Returns 1 on successful subraction, and 0 if nothing is done. */ Format block comments as:
/*
* Try to subtract r2 from r1. Only subtract if the resulting region
* is a rectangle, otherwise, do nothing.
* Returns 1 on successful subraction, and 0 if nothing is done.
*/
static int try_subtract_region(struct region *r1, struct region *r2)
{
/* If the regions are not intersecting each other, then
* we have nothing to do. */
if (!intersect(r1, r2)) return 0;
/* Same goes if r2 is covering the whole area. */
if (covering(r2, r1)) return 0; My opinion is that the two comments above are excessive, but this is
subjective, and depends on who will read your code. OS kernel hackers
would regard them as noice; in a homework project they show you
understand what you are doing.
/* Since region 2 is not covering the whole region 1, we can make
* certain assumtions, that is, if region 2 is more to the right, left * and bottom than region 1, it won''t cover the top and we
* remove the bottom part of region 1. */
/* r2 is wider */
if (r2->left <= r1->left && r2->right >= r1->right) {
if (r2->top <= r1->top) { /* r2 is covering the top part */
r1->top = r2->bottom;
return 1;
}
The two comments above are inconsistently placed: one before the if, one
after the if.
Also, in the style you are using, you should be formatting cascading if
.... else if sequences as:
} else if (...) {
--
Diomidis Spinellis
Code Quality: The Open Source Perspective (Addison-Wesley 2006)
http://www.spinellis.gr/codequality?clc
Diomidis Spinellis wrote:Edward Gregor wrote:I''ve written this code as a part of my program and I
wonder if you would mind looking at the
try_subtract_region and tell me if it well written.
Programming style is a highly subjective issue, and comments are bound
to generate flames, but here are some ideas.> struct region {
> int left, right;
> int top, bottom;
> };
Add a comment saying what this structure represents. Are the limits
inclusive or exclusive?
Sorry but what do you mean by inclusive and exclusive limits?
Thank you for your help!
Diomidis Spinellis <dd*@aueb.gr> writes:Edward Gregor wrote:I''ve written this code as a part of my program and I
wonder if you would mind looking at the
try_subtract_region and tell me if it well written.
Programming style is a highly subjective issue, and comments are bound
to generate flames, but here are some ideas.
[snip]
> /* Return 1 if the two regions are intersecting */
> static int intersect(struct region *r1, struct region *r2)
Split into two lines (the same throughout the program):
static int
intersect(struct region *r1, struct region *r2)
As you say, this is a highly subjective matter of style; on this
particular point my sense of style differs from yours.
I prefer to put the function type and name on one line. I see a lot
of code that puts the function name at the beginning of a line; I
*think* it''s because some tools happen to work better with that
layout. You could argue that the name of the function is the most
important piece of information, and that putting it at the beginning
of a line makes it stand out. That''s a valid point (hmm, what''s the
oppposite of a "strawman argument"?), but I find it artificial and
clumsy.
And I note that neither K&R nor the standard does this.
Of course if your organization requires this style, or if you''re
working on code that already uses it, you should follow it.
[snip]
Also, in the style you are using, you should be formatting cascading
if ... else if sequences as:
} else if (...) {
Again, my own style differs. In the style I prefer, an opening brace
goes at the end of a line (except for the outer brace of a function
definition), and a closing brace goes at the beginning of a line *by
itself*. For example:
if (condition1) {
/* ... */
}
else if (condition2) {
/* ... */
}
else {
/* ... */
}
I''m not sure there''s a really good reason not to put the "else" or
"else if" on the same line as the "}", other than that it''s what I''m
used to. Both styles are common, and both are internally consistent.
Another style point: I always use braces on conditional statements,
even when there''s only one statement being controlled. For example, I
write this:
if (condition) {
statement;
}
never this:
if (condition)
statement;
The only time I might break this rule is if I put the whole thing on
one line, which I rarely do:
if (condition) statement;
(This is a habit I picked up from Perl, which requires the braces, but
I find it a good idea for C as well.)
--
Keith Thompson (The_Other_Keith) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.
这篇关于代码质量的文章就介绍到这了，希望我们推荐的答案对大家有所帮助，也希望大家多多支持IT屋！
