22
votes
Accepted
REST API to list books, where null means "everything"
I personally think it's fine, but it helps to turn your thinking around from a "selector" to a "filter". Then, the default is to return all books, and you selectively filter down the set you're going ...
- 366
22
votes
Player data collection program
Classes
Why do you need a separate class for each game whose data you request? This is very inflexible, should you need to request the data for more or fewer games. Just use one class to represent a ...
- 465
16
votes
Accepted
Sending a C++ array to Python/NumPy and back
1. Review
It's not recommended to use using namespace std; — the problem is that this imports all of the identifiers from std, ...
- 49.2k
15
votes
If/else produce code duplication
Keep it like that. It is perfectly readable, easy to understand, everything is fine.
Naturally you could do some lambda trickery along the lines of:
...
- 4,722
13
votes
Parsing log files of HearthStone: The log reading API
The Design
A LogReader reads a log, filters the unwanted stuff and converts it into LogEntries. According to your design.
That'...
- 22.2k
13
votes
Accepted
Current ISS latitude / longitude position
Currently your code is not re-useable. To make it re-useable you should move the logic inside a function.
To do this the function we will have two functions here: ...
- 3,075
13
votes
League of Legends Summoner Analysis
find_game_ids is far more complicated than it needs to be. You have essentially two "counters", Idgame and ...
- 16.1k
12
votes
Accepted
Inspector Rubberduck
Having worked on a similar system, I came up with a very similar design. So either we're both doing something right, or we're both doing it wrong :)
Here are some things I would change, but obviously ...
- 16.1k
12
votes
Player data collection program
Abstracting classes
In your question you asked how to abstract classes so I thought I would show a concrete example of how this is done. Here is a minimal example of how you could abstract your game ...
- 221
11
votes
Accepted
Building a better Collection. Enumerable in VBA
Design
I think you need to break that class in two. Having instance members on a static class is pretty confusing and bug-prone.
I'd suggest Enumerable to be the ...
- 74.7k
11
votes
Parsing log files of HearthStone: The log reading API
I'll just go through this from top to bottom
...
- 25k
11
votes
API method to validate Facebook OAuth token
You have a lot of null checks (well, two at least), which I think leads to less readable code. Also, it shouldn't really be necessary to have them.
...
- 24.9k
10
votes
Key Press Handler
Looking at it from a general perspective...
Put your comments that describe functions in javadoc-style outside your functions. Comment blocks before functions are a common format when it comes to ...
- 22.2k
10
votes
Current ISS latitude / longitude position
The code is clean and understandable. There are only minor improvements that we can apply.
use more descriptive variable names: latitude_direction and ...
- 17.1k
10
votes
Accepted
General purpose replacement for enum with FlagsAttribute
int NextFlag(string category)
I'd expect this to throw when it runs out of flags.
I really really don't like that the first flag just happens to be ...
- 7,096
10
votes
Accepted
If/else produce code duplication
Assuming Update#getMessage() is of type Message and Update#getCallbackQuery() is of type <...
- 907
9
votes
Parsing log files of HearthStone: The log reading API
I am not convinced that your general structure is useful as it should be. I have been trying to untangle how your code us used, and what sequence of events happen in order for the code to run the way ...
- 96.8k
9
votes
General purpose replacement for enum with FlagsAttribute
Is this as extendable as I think it is?
Does it work for multi-bit flags? For instance
...
- 13.8k
8
votes
8
votes
Accepted
Async wrapper around public API
There's something odd about your constructors - normally chained constructors call the constructor with the most parameters, not the other way around:
...
- 74.7k
8
votes
Nested Sets in JavaScript
Interesting code,
the first thing that comes to mind is that forEachand map are slower than ...
- 32.7k
8
votes
API for SQLConnect Library
A few notes:
I feel like some of your properties should have the readonly attribute applied to them. Do we really want the API users to have the ability to change ...
- 21.7k
8
votes
Quiz Engine implementation
Good
Meaningful names are used for classes, methods, parameter and properties
using interfaces over implementation
using enums to signal a state
Not so good
Code isn't tested
Scope of ...
- 50k
8
votes
Accepted
8
votes
8
votes
Accepted
Normalize whitespace and describe string
I don't have much to say, but this stuff does look useful. The types are designed to be exposed to the world, so it's transparent to the consumer and could make the intentions clear.
There is one ...
- 7,096
8
votes
Player data collection program
The other reviewers have already explained how you should re-organize your classes, but I want to expand a bit on how you create them and how it can save you a lot of typing.
So we assume that you use ...
- 408
7
votes
Building a better Collection. Enumerable in VBA
You have some nested if statements that I think you could un-nest a little bit
...
- 28.5k
7
votes
Quiz Engine implementation
A few points on top of @Heslacher's excellent review.
Another violator of the single responsibility principle is the MoveToNextQuestion method, since it does two ...
- 107k
Only top scored, non community-wiki answers of a minimum length are eligible