0

Recently I was going through several small exercises in development. In one of them I was asked to convert a string to a number in VB .NET respecting the current locale. So I did :

Imports System

Public Module Aplicacion
    Public Sub Main()
        Console.WriteLine(Numero("Hello World"))
        Console.WriteLine(Numero(2))
        Console.WriteLine(Numero(.24))
        Console.WriteLine(Numero(2.4))
    End Sub
    Function Numero(input as String) AS Double
        Try
            return Convert.ToDouble(input)
        Catch Ex AS Exception
            return 0    
        End Try
    End Function
End Module

The teacher corrected this by telling me that "although it works, the right way is to declare a variable with the same type I am going to return, calculate its value in the body of the function and at the end return this variable".

Is there really any gain in doing that against doing it directly as I did? Maybe some compilation bonus or an improvement in performance?

Matias Barrios
  • 4,674
  • 3
  • 22
  • 49
  • 1
    Your teacher is wrong. (And "the right way" is subjective) – Dai Apr 28 '20 at 01:46
  • 1
    There is no performance penalty because even the most basic of optimizing compilers made in the past 30 years will elide insignificant variables. – Dai Apr 28 '20 at 01:48
  • 3
    But if performance really matters, you shouldn't be using exceptions at all (exceptions are expensive!). Instead use `Double.TryParse` and explicitly pass-in `CultureInfo.CurrentCulture` to indicate your intent of respecting the user's current culture settings. Avoid using the `System.Convert` class entirely. – Dai Apr 28 '20 at 01:48
  • @Dai Thanks for your comment. One question, does your second comment mean that depending on the compiler, the performance *could* be even worse by declaring and insignificant variable and then returning? – Matias Barrios Apr 28 '20 at 01:51
  • 3
    It "could" be worse in the same way that you "could" make your car run slower by carrying an additional 1Kg of weight in your pockets when driving: **it's an utterly insignificant effect** that will be dwarfed by much more important things (by analogy, like carrying a 70Kg+ passenger with you in your car) - and in your case, using `TryParse` instead of `Convert` will have a much bigger impact. – Dai Apr 28 '20 at 01:53
  • 2
    The VB compiler emits a return variable if you do not declare and use one. This is an artifact of the way pre-Net VB Function syntax worked where you assigned the return value to an implicit variable named the same as the function name; note that this syntax is still valid. You can see the effect of this in the debugger if you step through the function and hover over the function name and see the value. – TnTinMn Apr 28 '20 at 02:52
  • 3
    @MatiasBarrios - The biggest hint I can give you if you're coding in VB.NET is to put `Option Strict On` at the top of your code. It'll force you to be a better programmer. Try it on your code now. – Enigmativity Apr 28 '20 at 03:08
  • @Enigmativity And add the `Microsoft.CodeAnalysis` code-analysis NuGet package to your project too. – Dai Apr 28 '20 at 03:17
  • 2
    It's not about performance. It's about coding style and best practice. Some people consider having a single exit point in a method to be the right way to do it. I used to be in that camp. I was never religious about it but I have, over time, more an more moved towards exiting a method as soon as possible. Having multiple exit points can lead to less indentation and thus more readable code. If your teacher wants you to have a single exit point and will mark accordingly then you should do it for the class but be aware that it's not the one right way. Just be logical and consistent. – jmcilhinney Apr 28 '20 at 04:29
  • Thanks all for your comments. They have been elightening. – Matias Barrios Apr 28 '20 at 04:36
  • @jmcilhinney I always hated enormous If blocks when I knew a simple `Exit Sub` of `Return` would simplify things but then I started reading "Clean Code" and it said single exit. Glad to hear you are of the same mind as far as exit points. – Mary Apr 28 '20 at 07:10
  • 1
    @Mary I suppose by the same logic Uncle Bob advocates never using break or continue inside a block either? https://stackoverflow.com/questions/4838828/why-should-a-function-have-only-one-exit-point - i'm a midcamper – Caius Jard Apr 28 '20 at 12:50
  • Makes no difference to performance. That variable in fact exists at runtime, whether or not you explicitly give it a name. The big, big difference about the teacher's advice is that it has a name. That makes the debugger a lot smarter about what is going on in your program. You can inspect it, look at its value change in the Locals and Autos debugger window. Tells you when your function misbehaves, at the point of failure. It got less essential when they added the "return value inspection" feature to the debugger in VS2013, except that it is too late. – Hans Passant Apr 29 '20 at 16:05

1 Answers1

0
Function Numero(input As Object)
    If input.GetType Is GetType(String) Then
        Return 0
    Else
        Return input
    End If
End Function

Sub Main()
    Console.WriteLine(Numero("$$$$325FK-.,ASL$$$"))
    Console.WriteLine(Numero(1))
    Console.WriteLine(Numero(1.5))
    Console.ReadKey()
End Sub
PSK1337
  • 41
  • 1
  • 7
  • Please explain what your solution does and how it works. At the moment it is not a great answer. – Eugene Obrezkov Apr 30 '20 at 10:40
  • He wanted to improve the Speed this Function using If Statements is the fastest Solution possible If string found return 0 else return Input thats the correct way to do it instead of him using Catch to return 0... – PSK1337 Apr 30 '20 at 11:58