1
\$\begingroup\$

The searching algorithm that I created is dependent upon the sorting algorithm (which many of you have seen in my previous question). I believe that the sorting algorithm can't be better (for beginner-level programmers like me) but I would like to know what you think about my descriptions for the searching algorithm.

I would appreciate your insight on the commented sections of my code (again, for the searching algorithm). And if there are any other changes I can make, please don't hesitate to let me know.

def sorting_algorithm(list):
    for index in range(1, len(list)):
        current = list[index]
        while index > 0 and list[index - 1] > current:
            list[index] = list[index - 1]
            index = index - 1
            list[index] = current
    return list


print("the sorted list is", (sorting_algorithm([45, 14, 1, 7, 98, 23, 102, 57])))

#SEARCHING ALGORITHM CODE:

def searching_algorithm(list, num):
    fir = 0                                        #the first number of the list.
    las = len(list)-1                              #the last number of the list.
    i = -1                                         #the index is at the last number of the list.
    while (fir <= las) and (i == -1):              #the first number is less than or equals to the last number and the
                                                   #index is at -1 (so the last number).
        mid = (fir + las) // 2                     #the middle number is equal to the first num divided by the last number
                                                   #but without the remainder part.
        if list[mid] == num:                       #if the middle number of the list equals the number
            i = mid                                #then the index is moved to the middle number of the list.
        else:                                      #if that isn't true then we move on to the following condition.
            if num<list[mid]:                      #if the number is less than the middle number of the list
                las = mid -1                       #then the index moves one to the right.
            else:                                  #if that isn't true either, then move on to the following condition.
                fir = mid +1                       #then the program will conclude that the number is the greater than
                                                   #the middle number.
    return i

print("your number is at index",searching_algorithm([1, 7, 14, 23, 45, 57, 98, 102], 23))
\$\endgroup\$
0
3
\$\begingroup\$
  • The code is overcommented. Most of the comments do not add any value. They explain what the code is doing, which is obvious; the good comment shall explain why and how the code is doing what it is doing. Some of them are outright wrong, e.g.

       #the middle number is equal to the first num divided by the last number
    

    Say what? Nothing is divided by the last number!

    In any case, try to avoid comments. A presence of an explanatory comment means the failure to express yourself in the code.

  • As mentioned in the comment (no pun intended), the code implements a binary search.

      (fir + las) // 2
    

    is all right in Python with its arbitrarily long integers; in most other languages, which deal with the native integers, computing fir + (las - fir) / 2 is indeed preferable.

  • Testing for i == -1 looks smelly. Most of the times i remains -1, and it only gets the meaningful value when the target number is found. When the condition

      list[mid] == num
    

    is satisfied, it is a time to break the loop, or even return i right away.

  • Detour.

    Returning -1 when the target number is not there is not right. All the work your function have done is suddenly discarded. Don't be discouraged: you are in the good company. bsearch from the standard C library makes the same blunder.

    It is very helpful to return an insertion point instead: where the target number would have been. See how bisect.bisect_left() does it right (ditto for C++ std::lower_bound).

\$\endgroup\$
4
  • \$\begingroup\$ Return -1 is quite bad, -1 is an index which points to the last item in a list \$\endgroup\$ Dec 2 '20 at 5:58
  • \$\begingroup\$ @vnp Thank you for your honest analysis. I really appreciate it and I will make sure to use this for future reference :) \$\endgroup\$ Dec 2 '20 at 15:37
  • \$\begingroup\$ @esker-luminous don't forget to accept an answer to close a question. Looking forward to more projects from you. :-) \$\endgroup\$ Dec 2 '20 at 15:55
  • \$\begingroup\$ I did NOT know I had to accept an answer to resolve the question. Thank you again @theProgrammer!! But I guess I'll have to wait a couple of days to choose which explanation is the best for my problem (or like which one helped me the best). But if there is only this one answer, then I'll accept it. \$\endgroup\$ Dec 2 '20 at 16:04
2
\$\begingroup\$

First off great attempt for a new programmer. I see that you have implemented the binary search for your searching algorithm and the insertion sort for your sorting algorithm.

For your commenting issue; you don't want to write word for word what each line of code is doing. Like fir = 0 # the first number of the list you don't need to write this as this is common sense, the same goes for i = -1 # the index is at the last number of the list. Another example of what you should NOT do is this: if num < list[mid]: # if the number is less than the middle number of the list- this may provide clarity to you, but for some people it's not necessary to see. So you should keep these kind of things for your own reference (wherever you're writing this code, be it IDLE, PyCharm etc.), not publish on platforms like these (where believe me, you will be critiqued).

You should maybe just add a comment at the top of a chuck of code explaining what that code is going to do. Remember to be concise rather than writing paragraphs about what each line of code is doing. Your program is quite small so it's a little hard to find various examples, but that's ok we can just use what we have.

For example:

#write here what this 'chunk' or 'block' of code is for in ONE sentence here
while (fir <= las) and (i == -1):              
        mid = (fir + las) // 
        if list[mid] == num:
            i = mid
        else:
            if num<list[mid]:
                las = mid -1
            else:
                fir = mid +1

And yes sometimes it's better to over-explain than under-explain but SOMETIMES you just need to put the most basic and raw facts out there (try not to give TOO much detail!).

Again, I commend you for great try (for a beginner) and hope this answer was able to give you some perspective as to how you should use comments!

\$\endgroup\$
2
  • \$\begingroup\$ Thank you for your insight! This was definitely helpful for how I should be commenting. Appreciate it :)) \$\endgroup\$ Dec 2 '20 at 17:53
  • \$\begingroup\$ @esker-luminous Glad I could help! \$\endgroup\$ Dec 2 '20 at 17:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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