Add shuf command

[shooby-d]

Usage: shuf -h | file_name [-o output_file] [-n lines] Files: shuf.c shuf.h shuffler.c vec.c

[ajcatoiu]

Hello shuf group here are my comments:

I like how you incorporated the Fisher-Yates shuffling algorithm into your project. The code is very well written and this looks like a very fun command to work on. I also like that you integrated a -h flag to the shuf UPROG for brand-new users including myself.

There is an issue though. There are some comments in the code, but minimal. I wished there was more documentation on your shuf command so that I can properly test the code on my own. When I ran the test_shuf UPROG on qemu, the shuffling algorithm seems to work fine but I am not sure what the first integer parameter in your shuf_start() function represents. I noticed that some of my lines in my input file were missing when I shuffled them. Is the the first integer parameter in shuf_start() determining number of lines that are being printed to stdout? I just wished there was more documentation so that I could understand what is going on in my output.

I think most of the code is very well written, but you can improve your handle_output() method in your user/shuf_start.c file. I think you can minimize the duplication in your code by having the entire switch statement in it's own function. That way, you can just call the function twice inside of handle_output() to minimize the amount of code you have to write.

Well done!

[knchan03]

Notes:

• I really like your test program. It is very simple, but is very clear what is being tested for each test case.
• Again, as mentioned above, more documentation would be better to understand what each parameter is.
• For example:

  uint sz = 0;

  I'm assuming this is the size of a buffer, but a comment above would make this much clearer. Also, I see comments in vec_push itself, but a comment over vec_push(vec, line); on line 24 would make it much easier to follow.

More comments in handle_output would be nice for each case in the switch statements, maybe even defining some return values to make the switch cases clearer

#define INPUT 1
#define OUTPUT 2
#define FERROR -1

Suggestion for a new feature:

Maybe it can be cool also to list shuf lines given in the command line through the -e flag

Input:

shuf -e A B C D E

Output:

D
A
E
B
C

Input with -n flag: shuf -e -n 2 A B C D E Output:

C
E

Overall, great start! Again, I like the test file, but I think adding a few more comments similar to how you do it in vec_push(vec_t vec, char string) in vec.c, maybe adding the new flag, and adding to the test file for the new flag would improve this program slightly. Good job!

[malensek]

Code review feedback is good, please make the changes (you don't need to add the extra feature though). shuf_start implementation should be moved to shuf.c -- that's where I'd expect to see the implementation, and as it stands it doesn't give us anything extra to have the files split up. Nice idea to make the shuffle algorithm a library someone else could use.

Also, no docs.