Skip to main content
added 32 characters in body
Source Link
BeetDemGuise
  • 4.2k
  • 12
  • 29

A binary search is a very nice solution to this problem and you implementation is quite simple and clean. The only issue I find with this code is your style.

Your function DuplicateBinaryFind is misleading because of its PascalCase namecould be named better. At first glance I thought you were creating a newCurrently the name feels backwards (objectIfNotNullCheck in your code asvs PascalCaseCheckIfNotNull is the convention for) and it also suggests its going to find a duplicate binary value. A better name would be something like objectFindDuplicateInteger names in C#. Useor camelCaseFindDuplicateValue for function names.

Also, your spacing is inconsistent:

  • You have a blank line after dup = DuplicateBinaryFind(...) in the true block of your if-statement, but not in the false block. This blank space isn't needed.

  • You have single spaces between the arguments (which is good) in the DuplicateBinaryFind call in the false block of you if-statement, but not in the true block.

  • Not quite an inconsistency, but you do not have spaces around conditional operators (i.e >, ==) and select mathematical operators. There should be single spaces around these to help improve readability:

     if(left == right)        
     int middle = (left + right)/2
     if(arr[middle] < middle)
    

    The space around mathematical operations is basically left up to preference, but try and make the statement as readable as possible by separating different 'levels' of the operation with space.

A binary search is a very nice solution to this problem and you implementation is quite simple and clean. The only issue I find with this code is your style.

Your function DuplicateBinaryFind is misleading because of its PascalCase name. At first glance I thought you were creating a new object in your code as PascalCase is the convention for object names in C#. Use camelCase for function names.

Also, your spacing is inconsistent:

  • You have a blank line after dup = DuplicateBinaryFind(...) in the true block of your if-statement, but not in the false block. This blank space isn't needed.

  • You have single spaces between the arguments (which is good) in the DuplicateBinaryFind call in the false block of you if-statement, but not in the true block.

  • Not quite an inconsistency, but you do not have spaces around conditional operators (i.e >, ==) and select mathematical operators. There should be single spaces around these to help improve readability:

     if(left == right)        
     int middle = (left + right)/2
     if(arr[middle] < middle)
    

    The space around mathematical operations is basically left up to preference, but try and make the statement as readable as possible by separating different 'levels' of the operation with space.

A binary search is a very nice solution to this problem and you implementation is quite simple and clean. The only issue I find with this code is your style.

Your function DuplicateBinaryFind could be named better. Currently the name feels backwards (IfNotNullCheck vs CheckIfNotNull) and it also suggests its going to find a duplicate binary value. A better name would be something like FindDuplicateInteger or FindDuplicateValue.

Also, your spacing is inconsistent:

  • You have a blank line after dup = DuplicateBinaryFind(...) in the true block of your if-statement, but not in the false block. This blank space isn't needed.

  • You have single spaces between the arguments (which is good) in the DuplicateBinaryFind call in the false block of you if-statement, but not in the true block.

  • Not quite an inconsistency, but you do not have spaces around conditional operators (i.e >, ==) and select mathematical operators. There should be single spaces around these to help improve readability:

     if(left == right)        
     int middle = (left + right)/2
     if(arr[middle] < middle)
    

    The space around mathematical operations is basically left up to preference, but try and make the statement as readable as possible by separating different 'levels' of the operation with space.

Source Link
BeetDemGuise
  • 4.2k
  • 12
  • 29

A binary search is a very nice solution to this problem and you implementation is quite simple and clean. The only issue I find with this code is your style.

Your function DuplicateBinaryFind is misleading because of its PascalCase name. At first glance I thought you were creating a new object in your code as PascalCase is the convention for object names in C#. Use camelCase for function names.

Also, your spacing is inconsistent:

  • You have a blank line after dup = DuplicateBinaryFind(...) in the true block of your if-statement, but not in the false block. This blank space isn't needed.

  • You have single spaces between the arguments (which is good) in the DuplicateBinaryFind call in the false block of you if-statement, but not in the true block.

  • Not quite an inconsistency, but you do not have spaces around conditional operators (i.e >, ==) and select mathematical operators. There should be single spaces around these to help improve readability:

     if(left == right)        
     int middle = (left + right)/2
     if(arr[middle] < middle)
    

    The space around mathematical operations is basically left up to preference, but try and make the statement as readable as possible by separating different 'levels' of the operation with space.