Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have a function:

Private Function getbyte(s As String, ByVal place As Integer) As String
If place < Len(s) Then
place = place + 1
getbyte = Mid(s, place, 1)
Else
getbyte = ""
End If
End Function

For extracting a character at a particular place in a string, how can I shorten this or any alternate for the same purpose?

share|improve this question
2  
Welcome to Code Review! Please place the appropriate language tag on your question. –  200_success Aug 3 at 8:16
1  
vb.net and vb6 are radically different languages. Which one are you using? –  RubberDuck Aug 3 at 11:38
1  
@ ckuhn203: The function which i had given in this question is properly work in vb.net and vb6 –  Sujith Karivelil Aug 3 at 11:40
    
I rollbacked the edit in which you formatted the code. The code, and all aspects thereof are subject for review. Changing the code after the question has been posted and you have received answers can invalidate (parts of) those answers. Please don't edit the code on the question once you've received answers. –  Pimgd Aug 8 at 9:03

2 Answers 2

up vote 5 down vote accepted

According to MSDN, this function is already implemented:

' Code from MSDN
'
Dim myString As String = "ABCDE" 
Dim myChar As Char 
' The value of myChar is "D".
myChar = myString.Chars(3)


A review of your code

  • Please indent your code.
  • I would choose another, more meaningful name for your function: getCharAt or getByteFromString. It is now clear that the function operates on a string.
  • I would use the clearer parameter name str instead of s.
  • "Compress":

    place = place + 1
    getbyte = Mid(s, place, 1)
    
    ' change to
    getbyte = Mid(s, place + 1, 1)
    

    The first version is definitely not wrong or ugly, but why use an extra line if we could express the same logic in the same clear manner?

  • You could use If to shorten your code:

    Private Function getbyte(str As String, ByVal place As Integer) As String
      getbyte = If(place < Len(str), Mid(str, place + 1, 1), "")
    End Function
    
share|improve this answer
    
What's the difference between Iif and If like this? I've seen Iif, but never If in this manner. –  nhgrif Aug 3 at 12:05
    
@nhgrif Iif always evaluates both operands in comparison to If which will either evaluate the second or the third parameter. See also this comment. –  ComFreek Aug 3 at 12:06
    
Ah, that's an important difference. Good answer. –  nhgrif Aug 3 at 12:08

First thing's first. Indent the code. Everything inside of Function...End Function should be indented one tab or four spaces. Same thing with code inside the If...End If.

Private Function getbyte(s As String, ByVal place As Integer) As String
    If place < Len(s) Then
        place = place + 1
        getbyte = Mid(s, place, 1)
    Else
        getbyte = ""
    End If
End Function

As for a simpler method for getting a byte at a particular place in a string, I think it's a wash for VB6, but there is an alternative. Create a byte array then return the byte at the index you want to retrieve. How this is done is different in VB6 vs. VB.NET.

VB6

The function to return a byte array in VB6 is StrConv().

Private Function GetByte(ByVal str As String, ByVal place As Integer) as String
    bytes() = StrConv(str, vbFromUnicode)
    GetByte = bytes[place + 1]
End If

I did not implement any checks on the place parameter, but it should check for both place < Len(str) and place > Len(str).

Note that I used ByVal for both parameters. You had only used it for one of them in the code above. By default, references are passed ByRef, so it's good practice to declare it so we know that functions won't have side effects and go changing s on us. Speaking of s, single letter parameter names are the devil in VB6. The IDE isn't smart enough to replace all instances of it like in VB.NET. Try to do a find and replace on "s" and see what happens. Opt for a longer more meaningful name. Even dsmvwlng string to str would be preferable over s.

I think the old VB6 conventions should be dropped in preference of the new VB.NET naming conventions. Methods should be PascalCased. Note that I changed getByte to GetByte.

VB.NET

There is a built in string function to do exactly what your function does. It's called GetChar. MSDN documentation.

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.