The Wayback Machine - https://waybackassets.bk21.net/20150919230510/http://codereview.stackexchange.com:80/questions/105039/finding-the-floor-value-in-an-array
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Problem:

Given an sorted array of N distinct integers, find floor value of input 'key'. Say, A = {1, 2, 3, 5, 6, 8, 9, 10} and key = 7, we should return 6 as outcome.

How can I optimize my solution?

     private static int CalclualteFloorValue(int[] _sortedarray, int l, int r, int key)
        {
            if (_sortedarray[l] == key)
                return -1;




            int m = -1;

            while (l<=r)
            {
                 m = (r+l)/ 2;

                if (_sortedarray[m] == key)
                {
                    return  _sortedarray[m - 1];

                }
                else if (_sortedarray[m] < key)
                {
                    l = m+1;
                }
                else
                {
                    r = m-1;
                }
            }


            return _sortedarray[m];

        }

  int _floorValue = CalclualteFloorValue(_sortedarray, 0, _sortedarray.Length - 1,7);
share|improve this question
    
Is it possible to use the Array.BinarySearch<T> method? Or you need to reimplement it yourself? –  Dmitry yesterday

4 Answers 4

In terms of speed, your solution is about as optimized as it can get. In terms of readability, your solution has much room for improvement. I prefer readability over speed. You should try to make your code as readable as possible, then, if you have performance issues, search for bottlenecks using a profiler and optimize them. The method body of CalclualteFloorValue can be made into a one-liner, which happens to be slower but is much more readable.

private static int CalclualteFloorValue(int[] sortedArray, int key)
{
    return sortedArray.Last(x => x < key);
}

If you wish to keep the speed of your original solution and increase readability, there are many improvements you can make:

  1. Have a consistent parameter naming convention. _sortedarray has an underscore, but the other parameter names don't. I prefer to only use underscores for variables declared within classes but not within methods.

  2. _sortedarray should be lowerCamelCase. That is, sortedArray.

  3. If l will always be zero and r will always be array length - 1, declare them in the method body. Don't make the method caller do unnecessary work.

  4. Use descriptive variable names instead of single letters.

The beginning of the method will now look like this:

private static int CalclualteFloorValue(int[] sortedArray, int key)
{
    int left = 0;
    int right = sortedArray.Length - 1;
share|improve this answer

You use m, r and l as your variable names.

I suggest spelling the name in full, not just the starting letter, like middle, start, end (or middle, right, left)

share|improve this answer
1  
This does not answer the question about method optimization. This is better suited towards being a comment. –  Will Sherwood yesterday
2  
@WillSherwood Reviewers may comment on any aspect of the code. –  Caridorc 14 hours ago

This is actually a standard problem. If you're doing CP, heed what others have said, but if you're doing development, there's a function called lower_bound. Use that.

share|improve this answer

You're using -1 to indicate that the floor of the key is not in the array, but what if -1 is the floor of the key? Conflating indices and array elements in the return value is going to cause trouble.

The floor of x is the greatest value less than or equal to x.

Also, what happens when key < sortedArray[l]?

For the array 1, 2, 3, here are the results I would expect, and the actual results:

key    expected    actual
0      (no floor)  1
1      1           -1
2      2           1
3      3           2
4      3           3
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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