The code reviewer requested changes in a piece of code similar to this:

system("grep foo /tmp/bar")

or this:

res, err, st = Open3.capture3("grep foo /tmp/bar")

The reviewer said:

Just read the file natively in Ruby, no need to fork a sub-command.

Why does the reviewer believe it is better to use Ruby's native functions like for example File.read even though there is no untrusted data on the sub-command call?

CodePudding user response：

The best thing would obviously be to ask the reviewer, not us. But I can speculate.

• Calling out to a subshell is less efficient, plain and simple. It requires an additional process, whereas File.read runs in the current process.
• Your code is now OS-dependent. Windows, for instance, does not have a grep command, but File.read will work on any OS that runs Ruby.
• At a glance, a subshell makes me think something funny is going on. If I see File.read, I expect that in Ruby code. If I see a system call, I begin to suspect something strange is going on, causing me to doubt the nature of the code.

For what it's worth, grepping in Ruby is still very simple. In fact, it's literally called grep.

File.readlines("/tmp/bar").grep(/foo/)

If I encountered the grep code in the question in a code review, I likely would have flagged it for the reasons above.