Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

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;
}`
share|improve this question

closed as off-topic by svick, abuzittin gillifirca, Mat's Mug, 200_success, rolfl Dec 29 '13 at 23:12

If this question can be reworded to fit the rules in the help center, please edit the question.

    
You will need to post the contents of your do...while for this to make any real sense. –  Ant P Dec 29 '13 at 17:17
5  
This question appears to be off topic, because the code doesn't compile. –  svick Dec 29 '13 at 17:42

3 Answers 3

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, like while (condition) { ... }, so you know what you're getting yourself into when you're reading the code from the top to the bottom.
  • doesn't have a boolean type. There's System.Boolean and its language alias bool. Did you mean ?
  • Single-letter variable names are evil. Single-letter identifiers that use a lowercase "L" are just plain careless. Use descriptive names.
share|improve this answer

(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...)

share|improve this answer
  1. check is not a good name for the function as it does not say what it checks. It returns false if all numbers below row are present and true otherwise. So a better name might be numbersAreMissing.

  2. 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;
    
  3. 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.

share|improve this answer

Not the answer you're looking for? Browse other questions tagged or ask your own question.