2
\$\begingroup\$

So my friend told me he just did 1 while and a bunch of if-else with only using 3 arrays to get the same output as mine code does.

So I wonder if I just make this unnecessarily complicated. I have created 5 arrays here.

What's your opinion on this?

#include <stdio.h>
#include <fcntl.h> //for open
#include <unistd.h> //for close

char buf[1024], templine[512], currline[512], outputline[1024];

int strcmp(const char *p, const char *q) {
  while (*p && *p == *q)
    p++, q++;
  return (unsigned char)*p - (unsigned char)*q;
}

char to_lower(char chr){
  if ( chr >= 65 && chr <= 90 && chr != '\n')
    chr += 32;
  return chr;
}

void uniq(int fd, int count, int duplicate, int nodifcase){
  int readchr, i1, i2, i3, i4, i5 = -1;
  //i5 declare to -1 because the group array index should not increment when templine compares with empty currline(only once)
  int elemc1, elemc2, elemc3, group[1024] = {[0 ... 1023] = 1};
  //group array is to store grouping elements, every line exist at least once, thus contains 1024's 1s.
  elemc1 = 0;
  elemc2 = 0;
  elemc3 = 0;

  while ((readchr = read(fd, buf, sizeof(buf))) > 0){
    for (i1 = 0; i1 < readchr; i1++){
      elemc3++;//track characters
      if (buf[i1] != '\n'){
        if (nodifcase){
          templine[elemc1] = to_lower(buf[i1]);
        }
        else{
          templine[elemc1] = buf[i1];
        }
        elemc1++;//keep templine index starts from 0
      }
      else{ //init when reach newline
          if (strcmp(templine, currline) != 0){ //if they r't equal
            for (i2 = 0; i2 < elemc1; i2++){
              currline[i2] = templine[i2];
              elemc2++;//count uniq line length without \n, when uniq line is found
            }
            i5++;//count groupings
            elemc2++;//to add \n
            i4 = elemc3-6;//buffer "pointer"
            for (i3 = (elemc2-elemc1-1); i3 < elemc2; i3++){
              outputline[i3] = buf[i4];
              i4++;
            }
          }//default uniq
          else{
            group[i5]++;//increment when two strings are the same
          }

        elemc1 = 0; //the element counter here is to make sure every new reading stream into templine starts from [0]
      }//most outer else
    }//most outer for loop
}//1st while loop

int line = 0, i, j = 0, newlinec2 = 0, newlinec1 = 0;
  if (count){
    for (i = 0; i < i5+1; i++){
      printf("%d%c",group[i],' ' );
      for(;;line++){
        printf("%c", outputline[line]);
        if (outputline[line] == '\n'){
          line++;
          break;
        }//if
      }//2nd for
    }//1st for
  }//if
  else if (duplicate){
    for (i = 0; i < i5+1; i++){
      newlinec1++;
      if (group[i] > 1){
        for (;;j++){
          if (outputline[j] == '\n'){
            newlinec2++;
          }
          if (newlinec2 == newlinec1-1 && outputline[j] != '\n'){
            printf("%c", outputline[j]);
          }
          else if (newlinec2 > newlinec1){
            printf("\n");
            j++;
            break;
          }
        }//inner for
      }//if
    }//outer for
  }//else if
  else{//default uniq printing
    for(i = 0; i < elemc2 ; i++){
      printf("%c", outputline[i]);
    }//for
  }//else
}//void


int main(int argc, char *argv[]){
  int fd, count, duplicate, nodifcase;
  char c;
  count = 0;
  duplicate = 0;
  nodifcase = 0;

  if (argc <= 1) {//e.g. cat README.md | uniq
    uniq(0, 0, 0, 0); //uniq is the only one argumt
            //the 0 is standard input for file descriptor, the std-I connect to std-O
            //std-O in this case is the result from cat README.md
    return 0;
  }else{
    while((++argv)[0] && argv[0][0] == '-'){//probably should use getopt() instead of this
    while((c = *++argv[0])){
      switch(c){
        case 'c':
        count = 1;
        break;

        case 'd':
        duplicate = 1;
        break;

        case 'i':
        nodifcase = 1;
        break;

        default:
         printf("%s%c%s\n","Option ", c, " is unknown." );
         printf("%s\n", "Usage: uniq <-c -d -i>");
        break;
      }//switch
    }//inner while
  }//outer while
  if (duplicate == 1 && count == 1){
    printf("invalid usage.\n");
    printf("usage: uniq [-c | -d] [-i]\n");
    return 0;
  }
  if ((fd = open(argv[0], O_RDONLY)) < 0){ // O_RDONLY = 0
      //open() establishes a connection between file descriptor and the file
      //int open(const char *path, int oflag, ... );
      //the path pointer points to a pathname naming the file
      printf("uniq: cannot open %s\n", argv[0]);
    }
    uniq(fd, count, duplicate, nodifcase);
    close(fd);
  }//else
  return 0;
}//main

\$\endgroup\$
9
  • 1
    \$\begingroup\$ Why did you reimplement strcmp? \$\endgroup\$
    – Reinderien
    Oct 6, 2021 at 21:00
  • \$\begingroup\$ @Reinderien So I ran this code in my MacOS's terminal, and I need to write the strcmp function by myself to make it work. This is the version to run in mac, my code is actually for xv6, so in xv6's environment I will remove the strcmp() & make other changes.. \$\endgroup\$
    – 123 456
    Oct 6, 2021 at 21:03
  • \$\begingroup\$ Welcome to CR! What's uniq's specification, exactly? \$\endgroup\$
    – ggorlen
    Oct 7, 2021 at 0:18
  • \$\begingroup\$ @ggorlen so if you have macbook you can test it out from your terminal. It's a function to remove duplicates. my uniq() is pretty shallow, only have -c (count duplicates) -i (ignore case differences) and -d (print duplicates) I think mac has -u and something else \$\endgroup\$
    – 123 456
    Oct 7, 2021 at 1:37
  • \$\begingroup\$ Why did you reimplement tolower (badly)? \$\endgroup\$ Oct 7, 2021 at 8:03

2 Answers 2

3
\$\begingroup\$

How many buffers do you need?

So my friend told me he just did 1 while and a bunch of if-else with only using 3 arrays to get the same output as mine code does. So I wonder if I just make this unnecessarily complicated. I have created 5 arrays here.

A straightforward implementation of uniq would only need two buffers: one to read the current line into, and one to hold the previous line. Then you can use strcmp() or strcasecmp() to compare the two lines and decide whether it was a duplicate or not. You don't need a separate output buffer, you just print the previous line buffer whenever you detect that the current line is different.

However, you can even do it with just one buffer. Assuming this buffer already holds the previous line, you read from the input character by character, and check if it matches the character in the same position in the buffer. If all of them matched when you read to the end of the line, you know you have a duplicate. However, as soon as the character you just read was different, you print the contents of the buffer, and then overwrite it starting from the position you just found the mismatching character.

Processing a whole line at a time is much faster than doing it character by character, so I would recommend using two buffers.

Missing error checking

You are checking if opening the input file succeeded, but you didn't check whether you could read it completely without errors. Use feof() after the while-loop to check that the whole file was read successfully. There might also be errors while writing the output. After writing all the output, use ferror(stdout) to check if any error occured. If you detect an input or output error, make sure to print an error message (to stderr as Toby Speight mentioned), and also exit the program with a non-zero exit code, preferrably EXIT_FAILURE.

\$\endgroup\$
3
\$\begingroup\$

Instead of using open() and close() directly, we could use <stdio.h> for a more portable program.

Don't use reserved names for your own functions (here, a name beginning with str). I don't see why we have written this, though - just use the standard strcmp() from <string.h>.

Similarly, to_lower() can be replaced by the standard tolower() function (which isn't restricted to ASCII locale, though note that we probably want to use wide-character strings rather than multi-byte character strings, so consider converting the input and using towlower() instead).

We probably don't want to end the comparison when we reach a null character, so consider memcmp() (or wmemcmp()) instead.

The fixed-size buffers are problematic - consider a dynamic buffer, and reallocate (carefully) when necessary.

printf("%c", c) can be simplified to putchar(c). But it's probably better to operate a whole line at a time.

Error messages should go to stderr, not stdout.

When we provide options but no filename, the program fails, instead of reading standard input. That seems wrong to me.

Comments should be helpful. These ones tell us nothing we don't already know:

  if ((fd = open(argv[0], O_RDONLY)) < 0){ // O_RDONLY = 0
      //open() establishes a connection between file descriptor and the file
      //int open(const char *path, int oflag, ... );
      //the path pointer points to a pathname naming the file
\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.