Hot answers tagged

62

A different way is to let these pairs be represented by a class, which itself has a static factory method that will return null on failure. public final class Pair { private String key; private String value; public Pair(key, value) { // TODO: Add proper argument checking/throw exceptions. this.key = key; this.value = ...


51

Your first excerpt is fine, and it is better than your second. Your second excerpt is worse because of the deeper nesting. (Also, if you were to choose nesting anyway, I'd invert the conditions to put the shorter code branch first to reduce mental workload when reading the code.) Consider why the goto statement is considered harmful. Since it has the ...


46

Firstly, Python has a style guide and (unless you're given a different guide, in which case please provide a link to it) you should follow it. Your code generally follows it, but note that the imports are in the wrong order, it should be: from collections import Counter import re import sys import numpy as np Note alphabetical order, and a split between ...


33

Is there a faster way of accomplishing this? Would Multithreading/Multiprocessing speed things up? Any help or other feedback would be much appreciated! No, for the most part, multi-threading will make no difference for you. At some point the bottleneck should be the IO speed of reading 50GB of file content, and not the speed of the processing. You also ...


28

Always Profile Optimized Code. https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Rper-measure Use -O3 optimisation: https://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html Use a profiler: https://github.com/KDAB/hotspot https://en.wikipedia.org/wiki/List_of_performance_analysis_tools Reduce the duplication string inPath = ...


26

You said : I never use the variable but only the constructor to see if it doesn't throw an exception I would ask you, then why did you declare a variable at all ? You could have done : public boolean isNaN(final String string) { try { new BigDecimal(string); } catch (final Exception e) { return true; } return false; } ...


26

To complement the other answer: I wouldn't call it isNaN as it's by far not obvious, that you mean "BigDecimal-NaN". Actually, "isNaN" is confusing, even when we know, it deals with BigDecimal. NaN is a special double value, which you will NOT get when you try Double.parse("foobar"). Moreover, there's no Double#isNan(String). ...


26

import*: Please avoid importing all modules at all costs. Import each module you use separately Regex ಠ_ಠ Regex is evil, the worst. seriously. stop it. right now. kill python.exe right now and change it. You use beautifulsoup, a dom parsing library, literally a few lines later, but you choose to use regex. You sir, are evil. I would suggest looking ...


25

I find it strange that the zero-length line problem has not yet been pointed out: for(String line : lines) { if(line.charAt(0) == '#') { .... The code above assumes a non-empty line in all cases, otherwise you get IndexOutOfBoundsException. This is the sort of issue that should be identified and covered with Unit testing. This also leads ...


25

I see some things that may help you improve your code. Use all required #includes The program uses the bool type and assert() but doesn't have the associated required includes. Fix that by adding this: #include <stdbool.h> #include <assert.h> Use required #defines As noted in http://en.cppreference.com/w/c/io/fopen : As all bounds-...


24

Your problem is not with the loop construct - nor is it with GOTOs. Your problem is you are avoiding some tools that can make your code better. Here's an example. enum Validator { IsNotComment("Skipping commented line in file.") { @Override boolean isValid(String s) { return !s.startsWith("#"); } }, IsAPair(...


23

Additional C#6.0 Modifications How C#6.0 would you like it to be? You still have a few places you can use more C#6.0: I'm going to work bottom-to-top in your post: Usually, I don't support using expression-bodied members on void methods, but this class would benefit from a few of them: public class ExecutionContext { public ExecutionContext(int ...


22

You do not need HTML parsing at all. OEIS has a nice JSON output format. https://oeis.org/search?fmt=json&q=id:A000045 So the core functionality of your program can be written as something like import sys import urllib2 import json f = urllib2.urlopen("https://oeis.org/search?fmt=json&q=id:%s" % sys.argv[1]) doc = json.loads(f.read()) comment = ...


22

Good work! After a quick first pass, here's the main items I've found, in no particular order. I'm sure there's more to find in there, but these should improve your code a lot: overuse of explicit Edit: see comments below, this not not correct in every single scenarios. explicit is only useful on constructors with a single non-default parameter. adding ...


22

Without sacrifying something, you can probably gain the most (wall time) by using a hint such as posix_fadvise(POSIX_FADV_WILLNEED). Or, if portability is not paramount, something like readahead (Windows calls that function PrefetchVirtualMemory). Be sure to read the docs and watch for words like "blocking". The reason for wanting to prefetch is that while ...


22

Overview Akki has done a fine job on the review. Some things I want to emphasize: You pass things by value rather than using const references. vector<string> splitStr(string const& line, const char delimiter = ',') ^^^^^^ otherwise you are copying the line. string makeCSVLine(vector<...


20

Turning my comment into an answer by request. You should be able to do what you want by simply using the functionality already built into LibGDX. Caveat: I have not tried this, I just know that the class exists and it should be possible to extract the data you want from it, so: some assembly may be required on your part. LibGDX has functionality for ...


19

Here is a far and away superior solution: use java.util.Properties to handle your configuration file. Properties supports a key=value syntax that is very similar to the one you are trying to parse. It's up to you to decide whether that syntax is close enough to handle any existing configuration files you might have, and whether the validation and logging ...


19

I wouldn't both prefixing 'internal' functions with an underscore. Instead, just put this in a module, and only export the functions you want to export. If you care about following the node style guide, you should know that node suggests ignoring Crockford's fetish for one var declaration. Note: this is pretty controversial. I'm not a fan of var ...


19

C style Well, the first thing I have to say is that your code is very C-like. The first action here would be to move all those loose functions and globals into two C++ classes, Lexer and Parser. Another C vice of your code is to declare variables on top of a function. lex() is champion at that, with 18 variables piled at the top of the function. Absolutely ...


19

Is there a better way to do this? Well, you have dynamic dispatch and yet another scattered island of memory for every single token. That is massively inefficient. A simple way to more efficiency is writing your own mini-VM for executing expressions. You will still only have to parse once, but now all the memory is in a single compact chunk which will be ...


18

I've hesitated for quite a while, but decided there are a few points worth commenting on. The first and most obvious is that (by design) this simply doesn't do much. It can log the types of statements encountered in the JSON you give to it, but that's all. To be of much real use, you'd normally want to create something at least vaguely AST-like, storing the ...


18

I'd rewrite it with three main changes: Keep main() minimal and improve the user interface. By the Single Responsibility Principle, it's a good idea to limit main() to just calling the primary function with the appropriate parameters. In this case, the functionality splits very cleanly. Being a Unix/Linux user, I would prefer to see tools that adhere to ...


18

1) I would save constants (doubleQuote, etc.) as fields, so they don't take up extra space in already fairly large method body. 2) I think your use of goto is fine. However you can also rewrite it without goto. At first glance it boils down to: var buffer = new StringBuilder(); var line = new List<string>(); foreach(var ch in csv) { var newLine = ...


18

There are a number of things you can do to improve your code. Use const references where practical The parameters passed to the functions can be sped up by passing them as const references instead of by value. Doing so tells both the compiler and other readers of the code that the passed parameter will not be altered, and allows for additional optimizations ...


17

Well, you could find a "Pythonic" way to skip around a file, but that still wouldn't make it good code, in my opinion. There are some fundamental issues with your approach that make it rather hackish: f.seek(0) means that the file isn't being processed sequentially, and creates extra work. You're hard-coding ano = 1977 when in fact ANO 1977 would be right ...


16

The technique you are looking for is called "Data Driven" programming: static char map[256] = {0}; // init all the data to 0. map['{'] = 1; map['['] = 1; map['('] = 1; map['}'] = '{'; map[']'] = '['; map[')'] = '('; for(char ch: data) { char action = map[ch]; if (action == 0) { /* do nothing */ } else if (action == 1) ...


16

longestword should be named using an interCaps naming convention. Also, replaced and final aren't exactly helpful variable names. However, if you were to have an intermediate result called words, it would be quite clear what that stands for. It's good practice to terminate all statements with semicolons, even though JavaScript allows them to be omitted. ...


16

Aside from the remarks of Frank (which are valid): C++ remarks Possible message rot if (i % INDENT_SPACES) { std::cout << "line " << line_counter_ << ": one indent is always 4 spaces long. Treating " << i << " spaces as " << i / INDENT_SPACES + 1 << "indents"; } int indents = (i + ...


Only top voted, non community-wiki answers of a minimum length are eligible