Problem
I have an integer list l
and a predefined integer value row (which is never manipulated inside the code). If value of row is 5, then I want the loop to exit if l
contains 1,2,3,4. If it is 3, then l
should contain 1,2 and so on for any value. I have devised a way of doing this, but since I mean to use this in an app, can someone tell me a better way of doing this?
do
{
}while(check(row,l))
boolean check(int row,list<int> l)
{
for(int i=1;i<row;i++)
{
if((l.contains(i))
continue();
else
return true;
}
return false;
}`
Solution
-
check
is not a good name for the function as it does not say what it checks. It returns false if all numbers belowrow
are present andtrue
otherwise. So a better name might benumbersAreMissing
. -
You can make your loop body a bit shorter by inverting the condition:
for (int i = 1; i < row; ++i) { if (!l.contains(i)) { return true; } } return false;
-
You should really think about whether or not this check is something you want to do on every loop iteration. Especially if
row
is a bit larger this gets wasteful. There are probably better ways to organize your data so this check can be avoided but that entirely depends on the actual problem you are trying to solve.
(I answered this question on StackOverflow, just copying it here…)
If you want to check if your list contains all numbers from 1 to (row-1), you could use a HashSet
like this:
var hashSet = new HashSet(myList.Where(item => item >= 1 && item < row))
This adds all “relevant” items of the list to the Set.
Since a HashSet
contains every item at most once, you can check the Count
of it to ensure, all numbers are present, like:
var check = hashSet.Count == (row - 1)
Regarding performance, this might be more efficient than your solution, since the list needs only to be iterated once (in your solution, you have row-1
iterations, one for every Contains
operation). And adding to a HashSet
is considered a O(1) operation.
Note: The major drawback of this solution is that its not immediately apparent what it does. So consider adding a comment to it.
Another, more readable approach would be to explicitely check, if all numbers are contained in the Set, like
var hashSet = new HashSet(myList);
var check = Enumerable.Range(1, row).All(number => hashSet.Contains(number));
If you consider row
to be a constant the asymptotical time will be the same: O(n) for constructing the HashSet
, and O(1)*O(1)=O(1) for the check itself (first O(1) for the constant number of “rows” to be checked, second O(1) for the Contains
function…)
In addition to what has been said already, I’ll add the following points:
do { ... } while (condition)
isn’t the most readable form of loop construct. If it doesn’t affect the number of iterations, you should prefer putting the condition at the top of the code block, likewhile (condition) { ... }
, so you know what you’re getting yourself into when you’re reading the code from the top to the bottom.- c# doesn’t have a
boolean
type. There’sSystem.Boolean
and its language aliasbool
. Did you mean java? - Single-letter variable names are evil. Single-letter identifiers that use a lowercase “L” are just plain careless. Use descriptive names.