Reputation: 195
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?
Upvotes: 1
Views: 46
Reputation: 70367
The best thing would obviously be to ask the reviewer, not us. But I can speculate.
File.read
runs in the current process.grep
command, but File.read
will work on any OS that runs Ruby.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.
Upvotes: 4