2
\$\begingroup\$

I want to substring a String from given start index and given substring length.

  1. if the string is empty then returns String.Empty
  2. if the string length is greater than start index then it substrings from given start index to given length...
  3. if the string length is less than start index then it returns from start index to last index

I just want to know if it can be improved and shorter with all these checks.

  1. if the string is null or empty
  2. if the start index is not less than zero
  3. if the start index is less than the length of given input string.
  4. if the given input string length is less than the desired substring length.
  5. if the given input string length is greater than the desired substring length.

Sample Input and Ouput for my program:

SplitEntity("", 1, 15)               =>   ""  
SplitEntity("abcdef", 0, 3)          =>   "abc" 
SplitEntity("abcdef", 3, 100)        =>   "def" 
SplitEntity("abcdef", 0, 100)        =>   "abcdef"  
SplitEntity("abcdef", -1, 100)        =>   ""

My function is below:

Public Function SplitEntity(entity As String, startIndex As Integer, 
                                       subStringLength As Integer) As String
            Dim spilttedString As String = String.Empty

            If (Not String.IsNullOrEmpty(entity) AndAlso startIndex >=0 ) Then

                If (entity.Length > startIndex) Then
                    If entity.Length >= (startIndex + subStringLength) Then
                        spilttedString = entity.Substring(startIndex, subStringLength)

                    ElseIf entity.Length < (startIndex + subStringLength) Then
                        spilttedString = entity.Substring(startIndex, (entity.Length - startIndex))
                    End If

                End If

            End If

            Return spilttedString

        End Function
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! I hope you get some good answers. \$\endgroup\$ – Phrancis Jun 29 '18 at 1:20
  • \$\begingroup\$ Is there a reason you're re-inventing the wheel? You're basically doing the same job as substring and usingsubstring to do it. \$\endgroup\$ – tinstaafl Jun 29 '18 at 1:42
  • \$\begingroup\$ i am not re-inventing.... just some checks before substring the given string so that it doesn't fail.. .Substring fails -> if the given input string length is less than the desired length... if the start index is less than the length of given input string... if the string is null or empty... Input and ouput for my program: SplitEntity("", 1, 15) will return "" SplitEntity("abcdef", 0, 3) will return "abc" SplitEntity("abcdef", 0, 100) will return "abcdef" SplitEntity("abcdef", -1, 100) will return "" @tinstaafl \$\endgroup\$ – khandakar Tareq Jun 29 '18 at 2:00
  • \$\begingroup\$ Ok. that's the kind of information that should be in your original post \$\endgroup\$ – tinstaafl Jun 29 '18 at 2:08
0
\$\begingroup\$

While I agree with @tinstaafl about using an extension method, I don't agree with using exceptions to control the program flow. IMO the best exception is an exception which doesn't happen if one can avoid it.


By using a guard condition like

If (String.IsNullOrEmpty(entity) OrElse startIndex < 0 OrElse entity.Length <= startIndex) Then
    Return String.Empty
End If  

you can return early and it removes two levels of indentation of your code which makes your code more readable. But wait, what should happen if subStringLength has a value <0 ? You should really think about this because it is a corner-case which isn't caught.


Using an ElseIf with the reverted condition of the If doesn't buy you anything. A simple Else is enough. Maybe the compiler optimizes this but in the worst case the code is checking the condition also.

If entity.Length >= (startIndex + subStringLength) Then
    spilttedString = entity.Substring(startIndex, subStringLength)

ElseIf entity.Length < (startIndex + subStringLength) Then
    spilttedString = entity.Substring(startIndex, (entity.Length - startIndex))
End If  

If you only check if entity.Length < (startIndex + subStringLength) you could assign entity.Length - startIndex to the subStringLength and call Substring only once.


Applying all of the mentioned points would look like so

Public Function SplitEntity(entity As String, startIndex As Integer,
                                   subStringLength As Integer) As String

    If (String.IsNullOrEmpty(entity) OrElse startIndex < 0 OrElse entity.Length <= startIndex) Then
        Return String.Empty
    End If

    If entity.Length < (startIndex + subStringLength) Then
        subStringLength = entity.Length - startIndex
    End If

    Return entity.Substring(startIndex, subStringLength)

End Function  

The only thing you need to check is what you want your method to do if subStringLength < 0.

\$\endgroup\$
1
\$\begingroup\$

for something like this I would suggest making it an Extension Method.

To that end I would suggest a name change as well. Perhaps Substring_NoException.

To make it complete I think having the same overloads as Substring would help.

As for reducing/eliminating the conditional checks, one option is to use a Try/Catch block and simply catch every exception that Substring would and return an empty string except for the length being too long.

It might look something like this:

Imports System.Runtime.CompilerServices

    ''' <summary>
    ''' Extension method for <c>Substring</c> to return empty strings instead of throwing exceptions
    ''' </summary>
    ''' <param name="startIndex">The zero-based character position of a substring in this instance</param>
    '''
    <Extension()>
    Public Function Substring_NoException(input As String, startIndex As Integer) As String
        Return input.Substring_NoException(startIndex, input.Length - startIndex)
    End Function

    ''' <summary>
    ''' Extension method for <c>Substring</c> to return empty strings instead of throwing exceptions
    ''' </summary>
    ''' <param name="startIndex">The zero-based character position of a substring in this instance</param>
    ''' <param name="length">The number of characters in a substring</param>
    '''
    <Extension()>
    Public Function Substring_NoException(input As String, startIndex As Integer, length As Integer) As String
        Try
            Return input.Substring(startIndex, length)
        Catch ex As ArgumentOutOfRangeException
            If ex.ParamName = "length" AndAlso length > input.Length - startIndex Then
                Return input.Substring(startIndex)
            End If
            Return String.Empty
        End Try
    End Function

usage is:

Dim testString As String = "abcdef"
Dim testString2 = testString.Substring_NoException(3, 100)

Returned value is:

testString2 = "def"
\$\endgroup\$
  • \$\begingroup\$ Thank you so much...this is looking much cleaner...i will try to implement this and check with my requirements... \$\endgroup\$ – khandakar Tareq Jun 29 '18 at 3:31
  • \$\begingroup\$ @khandakarTareq - I added usage notes in case it wasn't clear. \$\endgroup\$ – tinstaafl Jun 29 '18 at 3:59
  • \$\begingroup\$ @khandakarTareq - It won't throw an exception regardless of the length or startIndex. the conditional is there so that when the length is too long the rest of the string is returned. All other exceptions will return an empty string \$\endgroup\$ – tinstaafl Jun 29 '18 at 4:18
  • \$\begingroup\$ sorry..i misunderstood that... this is the problem with us , the beginner ..lol.. thanks.. \$\endgroup\$ – khandakar Tareq Jun 29 '18 at 4:19

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.