New answers tagged comparative-review
0
Ideas for 2:
preallocate res
don't use one_res, just a comprehensions
0
Jae Bradley raises a very valid concern:
The use of ownerDisjointSet.find() on the node found by owner….getNode(item) prevents the differences in root finding algorithms to have any effect.
To compound to the problem, union computation does *not use disjointSetRootFinder, but ownerDisjointSet.find().
From one step back, you
- try multiple inheritance ...
2
Logic
If you're using Java 8+, in getNode I think you can simplify the logic using Map#computeIfAbsent like this
return disjointMapSet.computeIfAbsent(item, () -> new DisjointSetNode<>(item));
Is there a reason not to implement equals / hashcode for the DisjointSetNode classes? Relying on reference equality seems less easier to reason about than ...
1
I'll add another answer, because the previous one was focused on making code readable.
string numberAsString = "1640";
if (Int32.TryParse(numberAsString, out int number)) {
Console.WriteLine($"Converted '{numberAsString}' to {number}");
}
This is ugly. And it is ugly because you are mutating some value inside another function. It ...
0
This statement is concerning to me:
To break it down - we are not only getting one value instead of 2 (c# returns boolean and creates int).
TryParse does not create an integer. If you tried to compile the following code:
static void Main() {
int.TryParse("123", out myval);
}
You will get the following error:
error CS0103: The name `myval` ...
4
None of these seem like a winner. I would do something like:
function withParsed(string, success, error) {
const number = parseInt(string, 10)
if(!isNaN(number))
return success(number)
return error(string)
}
withParsed(
'1234',
(number) => console.log(`This is a number: ${number}`),
(string) => console.log(`String $...
1
By separating the Country dto class from CountryService / CountryProvider / Countries allows us to address some concerns on the latter one.
Some design concerns:
The country names and codes are changing fairly rarely that's why storing them only once gives good performance if you want to frequently access them
They are not allowed to change (without ...
1
The term I would use is Provider as in ICountryProvider, and I would expect it to be provided to the classes that use it via dependency injection. Of course that doesn’t have to be the case, you could create the interface and apply it manually. But I would consider your first example a code smell as it would tie the concept of Country too closely with the ...
2
For both functions, consider lining up the numbers so the tabular nature is easier to read:
void fill_color_gcs(xcb_connection_t *c, xcb_window_t w,
xcb_screen_t *s, xcb_gcontext_t *col)
{
col[0] = color_gc(c, w, s, 0, 0, 0 );
col[1] = color_gc(c, w, s, 1, 1, 1 );
col[2] = color_gc(c, w, s, 1, 0, 0 ...
1
I think the way you applied the macro in this case is spot on. My only concern is the mixture of integer and floating-point values. I would recommend replacing it with something like this that also looks a bit neater:
void fill_color_gcs(xcb_connection_t * c, xcb_window_t w, xcb_screen_t * s,
xcb_gcontext_t * col) {
COLOR_GC(0, 0.00, 0.00, 0.00);
...
4
For me it looks like you create some kind of color table here.
In the first step I would make the function more flexible by giving the colors as parameter
void fill_color_gcs(xcb_connection_t *c, xcb_window_t w, xcb_screen_t *s,
xcb_gcontext_t *col, float *colorTable)
and then just loop through the table
for (int i = 0; i < 16; i++)
col[i]...
2
Are you sure you want the non-numeric case to result in None? Pandas more commonly uses NaN. Anyway.
You're missing an important test case: what do you want to happen for values with a non-zero post-decimal half?
Option 1 is broken because you're just substituting 0 for non-numeric cases.
Option 2 is non-vectorized due to the map, and will not catch 123a; so ...
Top 50 recent answers are included