Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mean value drawing #14

Merged
merged 7 commits into from
Dec 3, 2018
Merged

Mean value drawing #14

merged 7 commits into from
Dec 3, 2018

Conversation

Des-
Copy link
Collaborator

@Des- Des- commented Dec 2, 2018

Changed the drawing algo such that it can choose the permutation of the number of players in a way that minimizes the rating variance.

CARE: Needs more adjustment. After pull, please check if data field "nczone_draw_factor" is in the db and needs finetuning wrt factor. Currently 0.4 seems nice, however 0.0 is the old algo.

@Seltsamsel
Copy link
Collaborator

Looks good so far. Language entries for NCZONE_DRAW_FACTOR and NCZONE_DRAW_FACTOR_DESCR are missing. Please add them in language/de and language/en .

Matching the style we use for PHP
zone/draw_teams.php Outdated Show resolved Hide resolved
zone/draw_teams.php Outdated Show resolved Hide resolved
zone/draw_teams.php Show resolved Hide resolved
config/config.php Show resolved Hide resolved
acp/main_module.php Outdated Show resolved Hide resolved
adm/style/acp_nczone_draw.html Outdated Show resolved Hide resolved
zone/draw_teams.php Outdated Show resolved Hide resolved
zone/draw_teams.php Show resolved Hide resolved
@@ -182,4 +182,28 @@ private static function get_min_rating_of_all(match_players_list ...$players_lis
return $list->get_min_rating();
}, $players_lists));
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for this whole block: indent is 4 spaces, please adjust accordingly :)

$value += $list->get_total_rating();
$number += $list->length();
}
return $value/$number;
Copy link
Collaborator

@Zutatensuppe Zutatensuppe Dec 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

preferred: return $value / $number;

you also typehinted int as a return value, but this division does not always result in an integer. is this desired?

Copy link
Collaborator Author

@Des- Des- Dec 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, sry. In java (the only programming language I really know better) int / int is automatically casted to an int. The intention was that the result is implicitly casted to an int. (i.e. all digits of the float behind the "." are thrown away.) I hope php does that here?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Des- yes, that is what would happen in php too. i was not sure wether that was intentional here ^^

return $value/$number;
}

public static function get_abs_rating_variance(match_players_list ...$players_lists): int
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be awesome to have a unit test for this public method :)

deleted whitespaces...

Co-Authored-By: Des- <danielschmand@gmx.de>
@Seltsamsel Seltsamsel merged commit c62b047 into teheru:master Dec 3, 2018
@Des- Des- deleted the meanValueDrawing branch December 12, 2018 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants