7
Bug: array shrinks too much
The logic in shouldContract looks like it is intended to prevent the size from shrinking below INITIAL_CAPACITY, but it doesn't do that. For example, suppose the current table size is 16, then table.length == INITIAL_CAPACITY is false and MAXIMUM_LOAD_FACTOR * size * 4 < table.length can be true, so a resize is performed and ...
5
Just an unordered list of things that came to my mind:
You have two rehash() methods doing very different things, an int-returning one computing the index for a content integer, and the other void one doing the rehashing process.
You have (at least) two places where you compute the array index for a content integer, one being inside the add() method, the ...
4
A few things to consider...
Naming
Naming is important, code is read a lot more than it is written and anything that makes the code easier to read helps. One element of that is consistency, which is something that your test names don't have. Some of them start with test and some of them don't. Some of them are quite descriptive about what's being tested ...
3
This looks very nice, with well named variables and a thought out design, good class and method names. The code is probably only readable if you study the algorithm, but that's OK.
I'll make some remarks though, starting with HungarianSolver. I might need to post another answer to review the rest, 'cause there is a lot of code to handle.
The formatting is ...
3
Your nose is correct. What you smell is the effect of voids that mutate the state of the class, so you need to expose the internals.
First step would be to split initialization into a factory.
public class HungarianSolverFactory
{
public static HungarianSolver hungarianSolverSolve(int[][] costMatrix)
{
HungarianSolver result = hungarianSolver(...
2
IntHashTableCollisionChainNode is already nested within IntHashSet, making the IntHashTable part of its name redundant. Plus the fact that it's part of a "chain" is kind of implied by how it's used, it's not really a property of the class itself.
Consider perhaps just IntHashSet.Node.
IntHashTableCollisionChainNode can be private
You didn't ...
2
I'm not a fan of random testing in unit tests. They should give repeatable results to be useful as a build step.
As a module test, random runs are great, but you've only done half the job:
srand(seed), rand(), printf("Seed %u.\n", seed);
It's no use printing the seed that was used if there's no option to re-use that seed in a future run. We need ...
2
Ensure you cover the basics
You have a lot of tests that check the operations on already created TinyDIP::Images, but you lack tests to check that the creation and initialization of images works as expected. I would add something like this at least:
BOOST_AUTO_TEST_CASE_TEMPLATE(image_creation, T, test_types)
{
std::size_t size_x = 10;
std::size_t ...
1
Main Code
Readability
For me, whilst you've addressed some bugs since you're initial version it looks like the code structure within your IntHashSet has moved in a less readable direction. Whilst you started out having a contract method which was responsible for reducing the size of the table now all of this logic sits within your remove method. This makes ...
1
Performance
If one considers speed (which seems to be a goal since no safety net is put in place such as HashSet has), I'd say that you're in good spot, but some improvements can be done.
Checking contains inside of add and removes makes the code loop twice through the nodes when an element is absent. Consider checking later in your add and remove method.
...
1
The storage is inefficient on machines whose CHAR_BIT is larger than 8. It wouldn't be hard to use all the available bits in each char. For example:
{ /* Copy a contiguous subset of bits from `a` into the new array, `b`. */
const unsigned a = bit_offset / CHAR_BIT;
const unsigned a_bit = bit_offset % CHAR_BIT;
unsigned b, rest;
for (b = 0, ...
Only top voted, non community-wiki answers of a minimum length are eligible