C# binary tree loop trouble

Question

I'm making a loop for searching a value from the array. If I find it, the position will be printed, if it's not "no such number" will be printed. But, if I have several same numbers, I will receive only one and then I am over the loop. How can I get the control, if the same number in the array? My best tries end with endless loop or completed loop from the start.

static public void Searching(int[] arr)
{
    string x = "yes";

    Console.WriteLine("Enter please searching number");
    while (x == "yes")
    {
        bool found = false;
        int target = Convert.ToInt32(Console.ReadLine());
        int searchkey = target;
        int mid = 0, first = 0, last = arr.Length - 1;
        while (!found && first <= last)
        {
            mid = (first + last) / 2;

            if (target == arr[mid])
                found = true;


            else
            {
                if (target > arr[mid])
                {
                    first = mid + 1;
                }

            if (target < arr[mid])
            {
                last = mid - 1;
            }
        }
    }
    String foundmsg = found
                ? "Item " + searchkey + " was found at position " + mid
                : "Item " + searchkey + " was not found";
    Console.WriteLine(foundmsg);
    Console.WriteLine("would you like to find another number?");
    x = Console.ReadLine();

        }

    }

I'm guessing English isn't your first language. I'm honestly not sure why you're using a loop at all. If you convert your numbers array to a string, then use .IndexOf(inputNumber), you'll have what you want. Did I miss something on the language barrier? — CDove
– CDove, Commented May 15, 2017 at 14:03
You might want to reword your fist sentence as many find that word offensive. — juharr
– juharr, Commented May 15, 2017 at 14:03
I just edited it out (and also did code formatting etc.) but now there is a pending edit. My edit goes out the window -_- — EpicKip
– EpicKip, Commented May 15, 2017 at 14:06
@cdove no that's fine, i know i could use that function, but that's my class task, so i should use only loops for that. — Neeka
– Neeka, Commented May 15, 2017 at 14:06
At first glance your code looks ok to me. Can you include the input that's causing you to get an infinite loop? Edit: are you saying if you have the same number multiple times you want to output the position of each of the matching numbers? — aquinas
– aquinas, Commented May 15, 2017 at 14:07

Scott Hannen · Accepted Answer · 2017-05-15 14:23:10Z

1

If you're looking to return all of the positions in the array where a particular value is found then create a list (like List<int>) and as you find the number in the array, add the position to the list. That way you can search the entire array without breaking out of your loop.

Then when you're done executing there will be either one or more items in the list or the list will be empty. If there are items in the list display those. If there are no items in the list return the "not found" message.

For example,

var indexes = new List<int>();
for(var index = 0; index < source.Length; index++)
{
    if(source[index] == target) indexes.Add(index);
}

When you're done you'll have either a list of matches or an empty list.

Lots of nested loops can get confusing. You have an outer loop that's handling the Console inputs. One way to make that easier to read is to isolate part into a function. For example,

List<int> GetIndexesOfMatchingNumbers(int[] source, int target)
{
    var indexes = new List<int>();
    for(var index = 0; index < source.Length; index++)
    {
        if(source[index] == target) indexes.Add(index);
    }
}

You can call this function from your "main" function. It's functionally exactly the same but it reduces the amount of logic you have to follow when you read the code. That makes it easier to understand, even for the person writing it.

edited May 15, 2017 at 14:23

answered May 15, 2017 at 14:11

Scott Hannen

29.5k4 gold badges53 silver badges69 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Scott Hannen Over a year ago

As a rule of thumb I keep methods to 15 lines or less (not counting spaces or lines with only braces.) Whether we're new or experienced, reading our own code or someone else's, it just makes it easier for the brain to process. It's usually easy to read a short function and tell what it's doing. Then we give that function a clear name, and it's easy to tell what we're doing - both in the function itself and wherever we call the function.

Dmitrii Bychenko · Accepted Answer · 2017-05-15 14:14:29Z

You can try using Array.BinarySearch in order to find some item's index and then starting from the index move backward and forward:

  int[] source = new[] { 1, 2, 3, 4, 4, 4, 4, 5, 15, 20 };

  int searchkey = 4;

  int first = -1;
  int last = -1;

  int index = Array.BinarySearch(source, searchkey);

  if (index >= 0) {
    first = index;
    last = index;

    for (int i = index - 1; i >= 0; --i)
      if (source[i] != toFind)
        break;
      else
        first = i;

    for (int i = index + 1; i < source.Length; ++i)
      if (source[i] != toFind)
        break;
      else
        last = i;

    if (first == last)
      Console.Write($"{searchkey} found at {first} position");
    else
      Console.Write($"{searchkey} found at [{first}..{last}] positions");
  }
  else
    Console.Write($"{searchkey} is not found");

CDove · Accepted Answer · 2017-05-15 14:24:20Z

0

Don't loop this. C# has built in functions to help you.

static public void Searching(int[] arr)
    {
        string x = "yes";

        Console.WriteLine("Please enter a number for which to search...");
        int target = Convert.ToInt32(Console.ReadLine());

        string Response = "Result of search is:"
        +  arr.IndexOf(target) != -1 
        ? arr.IndexOf(target).ToString() : "Not Found";
       //indexOf finds the first index of obj. in array
        Console.WriteLine(Response);
     }

This should satisfy your "when I find one of multiples I receive only one" requirement. You'll always get the first occurrence.

If you need all instances, and use a loop, it's not all that different.

static public void Searching(int[] arr)
    {
        string x = "yes";

        Console.WriteLine("Please enter a number for which to search...");
        int target = Convert.ToInt32(Console.ReadLine());
        string result = string.Empty
        for(int i =0; i < arr.Length; i++)
              if (i = target)
              { result += i.ToString() + ", " }
        }
        if(result != string.Empty)
        {
           result = result.SubString(0, result.Length - 2);
           //removes extra space and colon
        } else {
           result = "Not Found";
        }
        Console.WriteLine("Result of search is:"
        + result);
     }

edited May 15, 2017 at 14:24

answered May 15, 2017 at 14:12

CDove

1,96010 silver badges21 bronze badges

4 Comments

aquinas Over a year ago

I believe 1) it's for school and 2) he's trying to do a binary search. IndexOf isn't a binary search.

Scott Hannen Over a year ago

IndexOf finds the first instance. He's saying that the number could appear in the array more than once and he needs to return the position for each one.

littlerufe Over a year ago

But this doesn't satisfy the requirement (hidden in the comments): "but that's my class task, so i should use only loops "

CDove Over a year ago

Yeah I was writing this answer before he added that bit.

Collectives™ on Stack Overflow

C# binary tree loop trouble

3 Answers 3

1 Comment

Comments

4 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

Comments

4 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related