3 ms·

Basically this whole thing is crazy. First, this can be done with one line in Python. Second, it's almost a page of C++. Third: if done in a 10 lines in pyth

by lqdc13 11y ago

Basically this whole thing is crazy. First, this can be done with one line in Python. Second, it's almost a page of C++. Third: if done in a 10 lines in python it is around 30% faster than the C++ version and 2x faster than the perl version. Here is my reference Python version that also counts the words in the same non-code golf kind of way: import sys from collections import defaultdict def wc(path): d = defaultdict(int) with open(path) as f: for line in f: for w in line.split(): d[w] += 1 return sum(d.values()) if __name__=='__main__': path = sys.argv[1] print(path, ':', wc(path)) EDIT: They are counting new lines correctly.

Veedrac 11y ago
Some quick points: * You need to loop over sys.argv[1:]. * You could use binary mode to avoid decoding (which the C++ avoids). * You could use collections.Counter() and its `update` method, which should be both cleaner and faster on Python 3: d = Counter() with open(path) as f: for line in f: d.update(line.split())
- lqdc13 11y ago
  Counter is a lot slower though because it uses a heap underneath.
  - Veedrac 11y ago
    It doesn't - it directly inherits dict. I'm wrong, though; the optimizations Counter applies only make it faster when each `update` takes an iterable of a large number of elements. This is unlikely to happen in our case. You can actually bypass the Counter wrapper and use its accelerator directly: from _collections import _count_elements def wc(path): d = {} with open(path, "rb") as f: for line in f: _count_elements(d, line.split()) return sum(d.values()) However, this uses implementation details and is as such bad code.
    - lqdc13 11y ago
      Yup, I just checked. They're only using heap for most common. Nevertheless, defaultdict == win for counting things like if we actually wanted unique words in this case.
- raiph 11y ago
  Fwiw, the same (I think) in idiomatic Perl 6: +$path.IO.lines.words
JadeNB 11y ago
Let's not judge Perl in general by the author's Perl code, which starts with the baffling step of taking a reference to the argument array, then wraps it in a list and passes it to a function that immediately shifts it off the list and de-references it …. Also, as Veedrac pointed out (https://news.ycombinator.com/item?id=9562039 https://news.ycombinator.com/item?id=9562039), it carefully hashes the words by frequency, only to render the effort redundant by simply summing all the frequencies. (Your Python does the same, I assume for fidelity to the original.) A better Perl version might look like: for my $file ( @ARGV ) { my $words; open my $fh, '<', $file or die; local $/; say "$file: ", scalar(split /\s+/, <$fh>), " words"; } EDIT: Changed an explicit count of the `split` list into an implicit count via `scalar`.
mmaldacker 11y ago
dictionary is not necessary, the C++ impl just counts the number of words. This can be written as: int word_count(const char *const filename) { std::ifstream file{filename}; return std::distance(std::istream_iterator<std::string>{file}, {}); }

by lqdc13 11y ago

Veedrac 11y ago

Some quick points: * You need to loop over sys.argv[1:]. * You could use binary mode to avoid decoding (which the C++ avoids). * You could use collections.Counter() and its `update` method, which should be both cleaner and faster on Python 3: d = Counter() with open(path) as f: for line in f: d.update(line.split())

lqdc13 11y ago
Counter is a lot slower though because it uses a heap underneath.
- Veedrac 11y ago
  It doesn't - it directly inherits dict. I'm wrong, though; the optimizations Counter applies only make it faster when each `update` takes an iterable of a large number of elements. This is unlikely to happen in our case. You can actually bypass the Counter wrapper and use its accelerator directly: from _collections import _count_elements def wc(path): d = {} with open(path, "rb") as f: for line in f: _count_elements(d, line.split()) return sum(d.values()) However, this uses implementation details and is as such bad code.
  - lqdc13 11y ago
    Yup, I just checked. They're only using heap for most common. Nevertheless, defaultdict == win for counting things like if we actually wanted unique words in this case.
raiph 11y ago
Fwiw, the same (I think) in idiomatic Perl 6: +$path.IO.lines.words

JadeNB 11y ago

Let's not judge Perl in general by the author's Perl code, which starts with the baffling step of taking a reference to the argument array, then wraps it in a list and passes it to a function that immediately shifts it off the list and de-references it …. Also, as Veedrac pointed out (https://news.ycombinator.com/item?id=9562039 https://news.ycombinator.com/item?id=9562039), it carefully hashes the words by frequency, only to render the effort redundant by simply summing all the frequencies. (Your Python does the same, I assume for fidelity to the original.) A better Perl version might look like: for my $file ( @ARGV ) { my $words; open my $fh, '<', $file or die; local $/; say "$file: ", scalar(split /\s+/, <$fh>), " words"; } EDIT: Changed an explicit count of the `split` list into an implicit count via `scalar`.

mmaldacker 11y ago

dictionary is not necessary, the C++ impl just counts the number of words. This can be written as: int word_count(const char *const filename) { std::ifstream file{filename}; return std::distance(std::istream_iterator<std::string>{file}, {}); }