如何改进以下代码

Question

我正在学习C,经过几个小时的努力,我终于解决了以下练习:

"编写一个程序,从两个文件中交替合并行,并将结果写入stdout.如果一个文件的行数少于另一个文件,则较大文件中的其余行应该简单地复制到stdout."

但是,我对代码不满意.我觉得我过于复杂,并且有一个更简单的解决方案.

如何改进此代码？

#include <stdio.h>
#include <stdbool.h>

int main (void)
{
    char file1[11], file2[11];
    FILE *input1, *input2;
    int c, d, i = 0;
    bool end_of_file1 = false, end_of_file2 = false;
    bool file1_newline = false, file2_newline = false;

    printf ("Enter the name of the two files to be merged,\
 separated by space: ");
    scanf ("%10s %10s", file1, file2);

    input1 = fopen (file1, "r");
    input2 = fopen (file2, "r");

    while ( end_of_file1 == false ) {
    if ( file1_newline == false )
        c = getc (input1);

    if ( end_of_file2 == true && end_of_file1 == false
                                         && i == 0 ) {
        putc ('\n', stdout);
        i = 1;
    }

    if ( c == '\n' && end_of_file2 == true )
        i = 0;

    if ( (c == '\n' && file1_newline == false) ||
               (c == EOF && file1_newline == false) ) {
        file1_newline = true;
        putc (' ', stdout);
    }

    if ( file1_newline == false )
        putc (c, stdout);

    if ( file1_newline == true )
        d = getc (input2);
    if ( d == EOF ) {
        end_of_file2 = true;
        if ( c == EOF )
        end_of_file1 = true;
    }
    if ( file1_newline == true && end_of_file2 == false )
        putc (d, stdout);
    if ( (d == '\n' && c != EOF) || end_of_file2 == true )
        file1_newline = false;
    }

    fclose (input1);
    fclose (input2);

    return 0;
}

Answer 1

算法问题

...让我们看看问题,而不是代码......

编写一个程序,从两个文件交替合并行并将结果写入stdout.如果一个文件的行数少于另一个文件,则应该只复制较大文件中的其余行stdout.

鉴于你应该处理线条,阅读整行似乎更好.为此,您应该使用fgets()或者getline()(尽管后者的可用性不如前者).

char line1[4096];
char line2[4096];

...

char *l1 = fgets(line1, sizeof(line1), input1);
char *l2 = fgets(line2, sizeof(line2), input2);

while (l1 != 0 && l2 != 0)
{
    fputs(line1, stdout);
    fputs(line2, stdout);
    l1 = fgets(line1, sizeof(line1), input1);
    l2 = fgets(line2, sizeof(line2), input2);
}

/* One file has reached EOF */

if (l1 != 0)
{
    fputs(line1, stdout);
    while (fgets(line1, sizeof(line1), input1) != 0)
        fputs(line1, stdout);
}
if (l2 != 0)
{
    fputs(line2, stdout);
    while (fgets(line2, sizeof(line2), input2) != 0)
        fputs(line2, stdout);
}

挑剔的风格

就个人而言,我不喜欢你对函数的括号周围空间的方式- K&R运营商之间的区分,如if和for那里是分开的关键字和表达的空间和函数调用里没有这样的空间.但这是一个风格问题,因此非常主观.

这些代码行提供了充足的弹药:

    bool end_of_file1 = false, end_of_file2 = false;
    bool file1_newline = false, file2_newline = false;

    printf ("Enter the name of the two files to be merged,\
 separated by space: ");
    scanf ("%10s %10s", file1, file2);

    input1 = fopen (file1, "r");
    input2 = fopen (file2, "r");

不要在一行上组合多个声明,尤其是在初始化时.

bool end_of_file1 = false;
bool end_of_file2 = false;
bool file1_newline = false;
bool file2_newline = false;

(但是你使用后缀1和2而不是'无后缀'和2获得加分.)

不要在带有反斜杠的行之间拆分字符串文字.这是一种非常古老的方式.自1989年以来使用字符串连接,标准(并修复语法).请注意,在反斜杠换行技术的许多缺陷中,它会破坏代码的缩进,并且非常容易受到编辑错误的影响.

printf("Enter the names of the two files to be merged,"
       " separated by space: ");

考虑一下fflush(stdout);之前的阅读.在实践中,它通常不是必需的,但值得考虑.请注意,用户可以在单独的行中输入两个名称; 那也行.我认为,将文件名限制为10个字符是相当简约的.你应该允许至少256个字符.你在格式参数中指定字符串的大小是好的,并且正确地执行了(at sizeof(array)-1,not sizeof(array)).更有用的程序设计可能会从程序的命令行参数中获取文件名,而不是提示用户输入名称.

始终测试结果scanf():

if (scanf("%10s %10s", file1, file2) != 2)
    ...something went wrong...

始终测试结果fopen():

if ((input1 = fopen (file1, "r")) == 0)
    ...something went wrong...
if ((input2 = fopen (file2, "r")) == 0)
    ...something went wrong...

更多代码

while ( end_of_file1 == false ) {
if ( file1_newline == false )
    c = getc (input1);

if ( end_of_file2 == true && end_of_file1 == false
                                     && i == 0 ) {
    putc ('\n', stdout);
    i = 1;
}

将循环体缩进一级(或者,在StackOverflow上,不要使用制表符).你是正确使用int的c(后来d).

循环中跟随的逻辑是......模糊不清.目前尚不清楚你在做什么.一般来说,你想尽快关闭EOF; 你在做那个测试之前等了一会儿.循环的主体对我来说是不可理解的 - 非常复杂的逻辑(好吧,它看起来很复杂;我怀疑底层逻辑很简单,但由于没有解释它的作用,它看起来很复杂).