Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A Standalone scope #393

Closed
VBAndCs opened this issue Mar 12, 2019 · 18 comments
Closed

A Standalone scope #393

VBAndCs opened this issue Mar 12, 2019 · 18 comments
Labels
LDM Review in progress LDM Review is in progress LDM Reviewed: Rejected LDM has reviewed and rejected

Comments

@VBAndCs
Copy link

VBAndCs commented Mar 12, 2019

Many times I get an error because I declare a variable inside a block, the declare another variable with the same name after the block. I always think that since the block-scope variable has expired, then it is legal to to reuse the same name afterwards.

If condition Then
   Dim x = 1 ' Error, because the outer scope contains a var with name x
End If

Dim x = 2 

I gave it some thought, and found if is reasonable since some loop or goto can take the execution back and cause the ambiguity. In fact this case can be checked, so if the two vars are not inside a loop, and there is no goto after the second can jamb before the first, so it is ok to use the two vars!

Anyway, I thought I can use something to create a special scope, like using {} in C#, so I did this:

If condition Then
   Dim x = 1
End If

With Nothing
  Dim x = 2
End With

The With Nothing creates that scope and works fine, but I think it seems exotic and not all programmers will think of it. another way is:

If True Then
  Dim x = 2
End if

In fact I am not sure which of the two blocks has better performance:
referencing nothing or checking a true condition. Either way, the perpoe of both blocks is not clear to the reader. Same can be said about Try:

Try
  Dim x = 2
catch

End Try

which can be better if we can get rid of the useless empty catch block (Can we?)
so I think it will be better to add a new scope block for this purpose:

Let
  Dim x = 2
End Let

or if you have a better suggestion.

@pricerc
Copy link

pricerc commented Mar 12, 2019

Don't re-use variable names in the same method. It's bad form, and a good way of getting yourself bitten in the @$$ when you're trying to find a bug in two years time.

If your method has that many blocks that you think you need to reuse variables, then you probably need to be refactoring into smaller methods.

We don't need to be redesigning VB to accommodate more poor coding practices; it already allows too many.

And we especially don't need more curly braces. I don't mind them for array initializers, but they should not have been use for the With initializer, when we already had a perfectly acceptable End With.

@rskar-git
Copy link

@pricerc are you the sort who would never do something like

For i As Integer = ...
' ...
Next
'...
For i As Integer = ...
' ...
Next
'...
For i As Integer = ...
' ...
Next
'...	

in the same method because i should be used exactly once?

@rskar-git
Copy link

Yeah, I gave something like this some thought, see #36.

In a nutshell,

Do
    Dim myAutoVar1
    Dim myAutoVar2
    ' etc.
    ' ...
    ' All done, end lifetime of myAutoVar*
End Do

The End Do works the same as Exit Do: Loop, and Exit Do and Continue Do would also still work as expected.

Got 4 thumbs down on that. I honestly thought it to be something nifty.

Oh well.

But as it turns out, the way that automatic variables are done in VB is quite different from C#. In VB, they all get allocated at the start of their respective method, no matter their scope or first use; and their lifetime will be the entire duration of the method's execution anyway.

FWIW, I find re-use of variables as more problematic than re-use of names. And in the case of names, I think the rule that the C#/VB team established is quite reasonable: One cannot shadow another variable (re-use its name) while it is still in scope within the method. (One can, however, still shadow a field of the enclosing class/module...)

The only other idea to consider may be to go with lambdas:

Call (Sub()
          Dim x = 2
      End Sub)()

But no doubt there will be detractors on that idea too.

@pricerc
Copy link

pricerc commented Mar 12, 2019

@pricerc are you the sort who would never do something like

For i As Integer = ...
' ...
Next
'...
For i As Integer = ...
' ...
Next
'...
For i As Integer = ...
' ...
Next
'...	

in the same method because i should be used exactly once?

probably not in the same method.

But it's easy enough to just

Dim i As Integer 
For i = ...
' ...
Next
'...
For i = ...
' ...
Next
'...
For i = ...
' ...
Next

And there's no mistaking that it's all the same i.

@pricerc
Copy link

pricerc commented Mar 12, 2019

If we were going to implement arbitrary scoping blocks into VB, it would probably need to Begin..End blocks, a-la Pascal. Or T-SQL. I have a little sympathy with that, since I cut my coding teeth on Pascal and do a lot of SQL.

But I think the 'modern' recommendation would be to move that block into its own method.

So rather than:

Sub FooBar(top As Integer) 
    ' This loop does foo
    For i As Integer = 1 To top
        DoFoo(i)
    Next

    ' This loop does bar
    For i As Integer = 1 To top
        DoBar(i)
    Next
End Sub

you'd have

Sub FooBar(top As Integer) 
    DoTheFoos(top)
    DoTheBars(top)
End Sub

Sub DoTheFoos()
    ' This loop does foo
    For i As Integer = 1 To top
        DoFoo(i)
    Next
End Sub

Sub DoTheBars()
    ' This loop does bar
    For i As Integer = 1 To top
        DoBar(i)
    Next
End Sub

@rskar-git
Copy link

@pricerc

Have a look at https://github.com/dotnet/roslyn/blob/master/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs

Check out the many faces of method in private void CompileNamedType(NamedTypeSymbol containingType).

At line 465:

MethodSymbol method = (MethodSymbol)member;

At line 560:

foreach (var method in AnonymousTypeManager.GetAnonymousTypeHiddenMethods(containingType))

At line 574:

MethodSymbol method = new SynthesizedStaticConstructor(sourceTypeSymbol);

At the code review, what would your recommended refactor be?

@rskar-git
Copy link

@pricerc

So rather than ... you'd have ...

The Sub definitions for DoTheFoos and DoTheBars are each missing their respective top As Integer parameter. (Happily a simple mistake the compiler would catch - and I'm guilty of flubs like that too.) But the bigger point is if you needed to share more than an integer or two of state between these DoThe* functions, things could get messy.

@VBAndCs
Copy link
Author

VBAndCs commented Mar 12, 2019

@rskar-git
We are in the same page, but I disagree on Do..End Do. Once I see Do, I assume it is a loop.. End do can't always be visible on the view port. It is confusing, and It is not good thing to recognize a statment from its tail not its head.
But, you gave me an idea:

Let
    Dim x = 2;
End Let

It is the best until now!

@VBAndCs
Copy link
Author

VBAndCs commented Mar 12, 2019

Another one that I forgot to mestion, is to allow:

With
    Dim x = 2;
End With

But I find Let..End Let better.

@VBAndCs
Copy link
Author

VBAndCs commented Mar 12, 2019

@pricerc
BASIC is for Biginners. Its beaty is that it allows us to do things easy and fast and sometimes dirty.
In some cases, you write similar blocks of code with small diferences. A copy, paste and replace is the fastest approch to take. It becomes an agony if you have to change variable names just because of the scobe. You are sure that these vars are temp and will not cause any problem later. I used the {} for scoping in C# smetimes, so I am sure I will need it in VB some day, at least when I am converting some C# code to VB. Auto converters will find it easy if there is a stand alone scope in VB to translate {} to.

@pricerc
Copy link

pricerc commented Mar 12, 2019

The Sub definitions for DoTheFoos and DoTheBars are each missing their respective top As Integer parameter.

I didn't get enough sleep last night. Also why I'm not feeling as diplomatic as usual today :)

But the bigger point is if you needed to share more than an integer or two of state between these DoThe* functions, things could get messy.

Possibly, but that doesn't change the argument that a method should do as little as possible, and do it well. If your method has gone over 15-20 lines, then refactoring is (probably) in order.

At least that's what the experts tell me.

Sharing complicated state is of course easier these days with tuples in the mix.

@pricerc
Copy link

pricerc commented Mar 12, 2019

@pricerc
BASIC is for Biginners.

No it's not. And that would be labelled hate speech in some circles :)

In some cases, you write similar blocks of code with small diferences. A copy, paste and replace is the fastest approch to take. It becomes an agony if you have to change variable names just because of the scobe. You are sure that these vars are temp and will not cause any problem later. I used the {} for scoping in C# smetimes, so I am sure I will need it in VB some day, at least when I am converting some C# code to VB. Auto converters will find it easy if there is a stand alone scope in VB to translate {} to.

I understand what you're trying to achieve. I even appreciate your frustration. I'm just pointing out that
you're proposing a significant change to the language in order to work around what some (many?) would consider a poor coding style (one method doing too much), when the simplest solution is to change your coding style to suit the language, and use any or several of the fairly trivial workarounds that are available.

In between my earlier post and this one, I was working on some C# code (trying to fix a bug in a tool I've cloned from GitHub). I found myself coding this:

 void FixTimestampColumns(DataGridView grid)
 {
        DataTable table = (DataTable)grid.DataSource;
      
        foreach (var c in grid.Columns)
        { ... }

        foreach (var c in table.Columns)
        { ... }
}

So I fully understand where you're coming from.

But I realised that there was a potential for me to get confused later on when debugging, so I changed it to:

 void FixTimestampColumns(DataGridView grid)
 {
        DataTable table = (DataTable)grid.DataSource;
      
        foreach (var gc in grid.Columns)
        { ... }

        foreach (var tc in table.Columns)
        { ... }
}

even though there was nothing in the language to prevent me doing it the way I had it.

One could even possibly argue that VB is doing us a favour by reducing the risk of accidentally using the wrong variable.

@pricerc
Copy link

pricerc commented Mar 12, 2019

At the code review, what would your recommended refactor be?

More methods. Over 200 lines for one method is too many.

I'm sure I read somewhere that compilers are amongst the worst bits of code for breaking 'best practice' for coding....

@VBAndCs
Copy link
Author

VBAndCs commented Mar 12, 2019

No it's not. And that would be labelled hate speech in some circles :)

I love VB.NET. Being for beginners because its simplicity doesn't mean it is not for experts too. But unfortunately, Ms decided otherwise 2 years ago! this is what .NET language strategy says:

An interesting trend we see in Visual Studio is that VB has twice the share of new developers as it does of all developers. This suggests that VB continues to play a role as a good, approachable entry language for people new to the platform and even to development.

entry language for people new to the platform and even to development is a fancy definition for beginners :)

@pricerc
Copy link

pricerc commented Mar 13, 2019

I've read that strategy before. But it's also now two years old; a lot has happened since then.

entry language for people new to the platform and even to development is a fancy definition for beginners :)

That's one interpretation.

Just because something is an 'approachable entry language' doesn't make it for 'beginners' who should move to a 'better' language once they're no longer 'beginners'.

That statement also contradicts years of Microsoft saying the VB is a 'first class .NET citizen'.

Unfortunately, there is a pervasive attitude (since VB was competing with C++) that real programmers don't use VB, but use C/C++/Java/C#/F#/LatestFadLanguage; that VB is for sissies and noobs.

That this attitude exists high in the .NET group at Microsoft is unfortunate. And short-sighted.

The truth is that real programmers continue to use VB because it's more productive to code in and easier to read years later.

I also question the 'statistics'. As a rule, I used to turn off the box that said 'send usage data to Microsoft'. I think the same is true of many corporate developers, many of whom code in VB. So I suspect the stats hugely underestimate the VB community.

@VBAndCs
Copy link
Author

VBAndCs commented Mar 13, 2019

I agree with you, but MS initiated a negative feedback that will decrease VB.NET popularity exponentially, since it decided to cage VB.NET in windows only, so the problem now is far beyond traditional language preferences. Now we can't write modern AP.NET MVC Core apps nor Xamarin apps with VB.NET, which most business depends on today!
Since you read that strategy, you can expect that all beginners along with most of VB.NET developers has shifted to C#. If this continues, VB will die in a decade with last keystroke of the last old VB.NET loyal developer!

@gafter
Copy link
Member

gafter commented Mar 14, 2019

We're not going to add a construct for a stand-alone scope.

@VBAndCs
Copy link
Author

VBAndCs commented Mar 26, 2019

@pricerc @rskar-git @gafter @srivatsn
A potential benefit of this suggestion arises here: #397 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LDM Review in progress LDM Review is in progress LDM Reviewed: Rejected LDM has reviewed and rejected
Projects
None yet
Development

No branches or pull requests

4 participants