Calculating a page size to execute a subset of jobs at a time

We have to execute a bunch of jobs , each have an ID and they are passed on to the processor . Since the processor can only work on a limited number of jobs (denoted by deleteBatchSizeMax in the code snippet),we are using a paged approach so that only a subset is picked up by the processor.To implement this we have a functionality which calculates the Page Size. It takes in the total number of jobs, and the max page size. The code handles the situation where the page size may not be an exact multiple , example 22 items have a deleteBatchSizeMax of 10. This translates to 22/10 so it gives us a page size of 3 where the jobs are executed in chunks of 10,10,2.
Hence wrote the below described function!

private int CalculatePageSize(int totalNumberOfJobs, int deleteBatchSizeMax)
=> totalNumberOfJobs % deleteBatchSizeMax != 0
? (totalNumberOfJobs / deleteBatchSizeMax) + 1
: totalNumberOfJobs / deleteBatchSizeMax;

I am hearing complaints that apparently its not readable and I should do the if-else block. I think that the variable names are well defined , code is well commented (omitted here) and self explanatory.

I wanted to know the feedback of the community.

Code Review Asked by user93068 on December 29, 2020

There are a few ways which this method could be made more readable.

The first if-statement is negated, making the control flow slightly more complicated; the statement can be negated (again) to clarify the flow:

private int CalculatePageSize(int totalNumberOfJobs, int deleteBatchSizeMax)
=> totalNumberOfJobs % deleteBatchSizeMax == 0
?  totalNumberOfJobs / deleteBatchSizeMax
: (totalNumberOfJobs / deleteBatchSizeMax) + 1;

Secondly, the statement is currently checking if totalNumberOfJobs is an even divisible of deleteBatchSizeMax. The computation totalNumberOfJobs / deleteBatchSizeMax is used three times. When (double)totalNumberOfJobs / (double)deleteBatchSizeMax doesn't contain a decimal place, then it implies totalNumberOfJobs % deleteBatchSizeMax == 0, and vice-versa.

To simplify the pattern, consider the following: when A divides B with no remainder, then divide it, otherwise divide it and add one. Therefore, Math.Ceiling can be used:

private int CalculatePageSize(int totalNumberOfJobs, int deleteBatchSizeMax)
=> (int)Math.Ceiling((double)totalNumberOfJobs / (double)deleteBatchSizeMax);

The Math.Ceiling function "rounds" up to positive infinity; so 4.5 -> 5, 4.00001 -> 5 and so on. This emulates the original loop condition.

Edit: the second double conversion is not required as it is upcasted to a double.

private int CalculatePageSize(int totalNumberOfJobs, int deleteBatchSizeMax)
=> (int)Math.Ceiling((double)totalNumberOfJobs / deleteBatchSizeMax);

Correct answer by alexyorke on December 29, 2020

I would go with following sample

1. Mare sure total and count are positive numbers

2. On next step we expect the pages result to be always positive > 1.

Divide the total number of item by the count. All divisions by total < count leads to 0 that is the reason +1 is aded to the result. By having total-1 we handle the case total = count and having + 1 will give are wrong result. Well this could be handled with few ifs but in the case you prefer a shorter version ...

private int pages(int total, int count){
if(total <= 0 || count <=0 ){
return 0;
}
return ((total - 1) / count) + 1;
}

Tests:

pages(0,10) -> 0
pages(1,10) -> 1
pages(10,10)) -> 1
pages(11,10)) -> 2
pages(101,10) -> 11

Answered by Traycho Ivanov on December 29, 2020

In a MathUtils class, define:

public static int DivCeiling(int num, int den) => (num + den - 1) / den;

And then, in your application class, define:

private int CalculatePageSize(int totalNumberOfJobs, int deleteBatchSizeMax)
=> MathUtils.DivCeiling(totalNumberOfJobs, deleteBatchSizeMax);

That way you can combine the short variable names of an abstract class for mathematical utilities (since the formula can be used in really many contexts) with the readability and simplicity in your application class.

You can also write extensive unit tests for the MathUtils class, focusing on the formula. And in your application code you only have to decide whether it is correct to use the DivCeiling function or not.

As usual for unit tests, pay attention to edge cases:

• num == 0 (always results in 0)
• den == 0 (exception, that's ok)
• num == int.MaxValue (overflow in my code, but converting the numbers to double feels inappropriate to me)
• den == int.MaxValue / 2 + 1, num = int.MaxValue / 2 + 2 (overflow)
• num < 0
• den < 0

To fix this, you could do the intermediate calculation using long instead of int. You just have to ensure that when converting the result back to int, that you don't cut off any significant digits.

Answered by Roland Illig on December 29, 2020

Related Questions

std::array and std::vector Type Arbitrary Nested Iterable Generator Functions Implementation in C++

1  Asked on February 2, 2021

10 Kinds of People

1  Asked on January 29, 2021 by martin-york

Generating product variants in Ruby

1  Asked on January 29, 2021 by dcangulo

Structuring code logic for events on laravel controller

2  Asked on January 28, 2021 by grey

Handling Circular References Without Recursion

2  Asked on January 27, 2021 by kittoes0124

vector<tuple > to map

1  Asked on January 25, 2021 by valerij

naming convention for atomic design

0  Asked on January 23, 2021 by irohitbhatia

An Implementation of Two Dimensional Plane as Monochromic Image Container with std::unique_ptr in C++

1  Asked on January 21, 2021 by jimmyhu

Cave explorer: using stack

0  Asked on January 21, 2021 by sherloock

Calculate the free electron concentration and drift speed in a segment of wire

2  Asked on January 18, 2021 by mode77

List of Happy Numbers in scala

3  Asked on January 18, 2021 by vikrant

Calculator Program improvements

2  Asked on January 16, 2021 by the

JavaScript async/await function performance improvement

0  Asked on January 14, 2021 by nmsdvid

Is this good c++ code for a pin/socket for a node editor?

1  Asked on January 12, 2021 by apoqlite

Arduino-based darkroom timer

0  Asked on January 12, 2021 by marcellothearcane

Finding the possible number of equal pairs in an array

1  Asked on January 11, 2021 by alaa-jabre

Python : adding a columns with count of missing values by row

1  Asked on January 10, 2021 by lcrmorin