-
Notifications
You must be signed in to change notification settings - Fork 66
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
Throw Expression #370
Comments
+1 . When I translate C# samples to VB with tools, I usually see compilation errors like this: _currentValue = If(newValue, Throw New ArgumentNullException(NameOf(newValue))) ' BC30201: Expression expected |
So that's a shortcut for: If newValue Is Nothing Then
Throw New ArgumentNullException(NameOf(newValue))
End If
_currentValue = newValue I'd say that what you're describing is a limitation of the translation tool you're using that doesn't know that C#'s traditional ternary statement has been overloaded with a new use-case, which is a hybrid operator-and-control-flow-statement. I kind-of see how it would work in C#, hi-jacking the ternary format, but I'm not a fan of mixing flow-control and assignment into one statement, and it looks to me like unnecessary obfuscation for not much return. Never mind that it would require changing VB's If operator into something different (as if there aren't already enough variants of If in VB). This would be very low on a list of enhancements to VB that I'd be interested in. |
How about write a function: Function IfNothingThrow(Of T As Class)(value As T, name As String) As T
If value Is Nothing Then
Throw New ArgumentNullException(name)
End If
Return value
End Function And you can call it like _currentValue = IfNothingThrow(newValue, NameOf(newValue)) |
@Nukepayload2 could you post the original C# code that produced
I want to test my translator to make sure it produces working code. |
@paul1956 namespace CreditCardFraudDetection.Predictor
{
public class Predictor
{
private readonly string _modelfile;
private readonly string _dasetFile;
public Predictor(string modelfile, string dasetFile) {
_modelfile = modelfile ?? throw new ArgumentNullException(nameof(modelfile));
_dasetFile = dasetFile ?? throw new ArgumentNullException(nameof(dasetFile));
}
}
} |
@Nukepayload2, the translation by two different tools gave two different code interestingly. First I used http://www.carlosag.net/tools/codetranslator/ which looks incorrect as below.
While http://converter.telerik.com/ gave an interesting additional support class code for C#'s throw implementation as below.
@paul1956, please share your tool's output. Could help us all. Also try to make your code translator as VSIX addon to VS. |
@pricerc : I don’t think they changed operators in c#, they changed throw. And yes, it does mix assignment with flow. But unlike C’s if (y=z){}, it’s adding control flow to assignment, not assignment to control flow. It’s not going to cause any any confusion or mistakes. And it makes it clear that if you can’t make the assignment that the current scope can’t complete. Your equivalent code doesn’t make that clear because the assignment happens 4 or 5 lines later. |
@rrvenki it a desktop application, that can do projects, folders and snippets. Source can be found here https://github.com/paul1956/CSharpToVB and it produces garbage for the code above, something I will fix this weekend. My goal was to be able to translate all of Roslyn with comments and reasonable formatting. What I do when VB can't do something directly that C# can, is try to see if I can get something logically similar and if that fails comment out the code and flag with TODO so I can continue. I am waiting for VB16 preview 2 to improve comment translation. Today I keep most/all of the comments, regions, directives but some of the comment are not near where they should be and some directives break VB code but in a very consistent way that a human can easily fix. With the addition of
I should be able to fix all the comments. If #357 gets implemented directive issues will be fixed. |
@rrvenki Here is what it produces after the fix Namespace CreditCardFraudDetection.Predictor
Public Class Predictor
Private ReadOnly _modelfile As String
Private ReadOnly _dasetFile As String
Public Sub New(modelfile As String, dasetFile As String)
If modelfile Is Nothing Then Throw New ArgumentNullException(NameOf(modelfile))
_modelfile = modelfile
If dasetFile Is Nothing Then Throw New ArgumentNullException(NameOf(dasetFile))
_dasetFile = dasetFile
End Sub
End Class
End Namespace |
As much as this may seem like a way to invite an abusive level of code golf, I can appreciate this as a means for inline-style values checking, particularly for nulls and out-of-range conditions. The But if @Nukepayload2's C# example could look more like
instead of
that looks a whole lot nicer on the eyes and brain. ==== This being VB, I wonder if we could instead go with having a
Hence, it could be
So I guess instead of
it could be
|
The other place this come up public static bool Parse(ReadOnlySpan<char> value) =>
TryParse(value, out bool result) ? result : throw new FormatException(SR.Format(SR.Format_BadBoolean, new string(value))); becomes Public Shared Function Parse(value As ReadOnlySpan(Of Char)) As Boolean
Dim result As Boolean = Nothing
If Not TryParse(value, result) Then Throw New FormatException(SR.Format(SR.Format_BadBoolean, New String(value)))
Return result
End Function @rskar-git for this to be useful it needs to support all the forms of throw, which is why I like the above approach. It is also very clear what is happening. It would be a little cleaner if VB supported creation of out variables and didn't require they be initialized (I think requiring initialization is being fixed in VB16) |
@paul1956, a side note, I think there's a missing
|
you say 'Tomato'... either way, the ways you can use the ? : construct has changed.
The argument about clarity is subjective, but the 4 or 5 lines later is just because I expanded the 'Then' clause. You could just use a single-line If/Then as demonstrated elsewhere, and then the assignment is on the very next line, which is what I usually do for guard statements. But I usually put all guard clauses together, and then the assignments afterwards. So @paul1956 's example becomes Public Sub New(modelfile As String, dasetFile As String)
If modelfile Is Nothing Then Throw New ArgumentNullException(NameOf(modelfile))
If dasetFile Is Nothing Then Throw New ArgumentNullException(NameOf(dasetFile))
_modelfile = modelfile
_dasetFile = dasetFile
End Sub I think it might be because I learnt to code at a time when clock cycles were important and optimizing was done by me, not the compiler. And so I still hate wasting clock cycles on needless assignments. But to each their own. I still don't like the mixing of flow and assignment; it still looks to me like unnecessary obfuscation and VB trying to be 'like' C#. For me, I value language stability and ease-of-mainteance over 'cool new tricks' or 'shortcuts'. So I would like VB's future to be about improving the compiler while keeping it an easy-to-master, easy-to-read language, for the benefit of the developers using it. I would prefer new features to be about new functionality and/or fixing anomalous or inconsistent behaviour, not new ways to do old things. For me, I don't think this proposal would achieve these. |
@pricerc : I wanted it in C# because I found it clear and useful in Perl (where it’s or die), so it’s definitely not trying to be like C# — it’s a syntax/idiom I find useful and readable. Now the fact that I use Perl may cause you to doubt that I understand what “readable” means... |
@rskar-git If #370 were available I would use it but there is a reasonable workaround. There are many items without any workaround that I would like to see first. Thanks I missed the Not. Right now I am the only one testing and without VS Enterprise my automated tests don't work. |
RE :Readable. For me, readable means readable by people who are not necessarily programmers. I have on rare occasions, over the phone, talked someone through editing a bit of VB code I appreciate the 'abbreviating' of common patterns where they are more likely to produce more maintainable (i.e. readable) code. E.g. auto-implemented properties. But (for me) this proposal adds unnecessary complexity - not that the construct in itself is complicated, but that it adds yet another way of doing something we can already do very simply. I don't see that:
is much of an improvement over:
The latter I can easily explain to a novice or lay person. The former would require a few more words of explanation. And then there's the fact that the latter is more flexible. While I won't be throwing my toys out of the cot if it's implemented; heck I might even use it. It's just that I think there are better things the compiler writers could be busy with.
|
I can't agree more.
crystal clear and self explanatory. |
Demo code: IO.vb If stream.FileExists Then
stream = stream.ReadAllText Or die("No content data!")
ElseIf Not stream.Match("http(s)?[:]//", RegexICSng).StringEmpty Then
stream = stream.GET Or die("No content data!")
End If |
@xieguigang : I would be totally welcoming of a Actually I would love it if |
C# introduced throw expressions in 7.0 after being requested in
dotnet/roslyn#5143, I would like to see the same thing in VB.Net.
The text was updated successfully, but these errors were encountered: