0

Take a look at this VB.NET code:

list = GeoController.RegionByCountry(country, language)
Region.allByLanguage(key) = list

In C#, I could write this in one line:

Region.allByLanguage[key] = 
    (list = GeoController.RegionByCountry(country, language))

Is there a way to make this a one-liner in VB.NET, like I can in C#?

EDIT: You all must need some sleep, or else you might be thinking a little harder.

Region.allByLanguage is a cache.

Here's the context:

Dim list As IEnumerable(Of Region)
Dim key = Region.CacheKey(country, language)

If Region.allByLanguage.ContainsKey(key) Then
    list = Region.allByLanguage(key)
Else
    list = GeoController.RegionsByCountryAndLanguage(country, language)
    Region.allByLanguage(key) = list
End If

Return list

How can you tell me that's not verbose code? Shudders.

Heck, if this was a C# team, I'd just write:

return Region.allByLanguage.ContainsKey(key) ? 
       Region.allByLanguage[key] : 
       (Region.allByLanguage[key] = GeoController.RegionsByCountryAndLanguage(country, language));
Improve this question
5
  • 4
    Even in C#, I almost certainly wouldn't do that... Commented Mar 1, 2011 at 8:18
  • Look at the context, Jonny boy. The Skeetster most definitely would do that, in this context, I hope. Really, other developers can't understand the blah = (blah2 = blah()) syntax? Who do you guys have to work with that this would be a readability issue? Poor fellas. Commented Mar 1, 2011 at 9:17
  • 1
    I hope you are kidding but I fear you are not. Writing code like this is like nesting shorthand-if's. So for example you write code like this (a > b? ((a > c) a : b ) : c). For the person writing the code, it's not hard to write, but it will need close examination for the reader to figure out what is going on. Commented Mar 1, 2011 at 9:25
  • 2
    And by the way, try to be a bit more respectful if you want to get any answers around here. It seems you have misunderstood length of code for ugliness of code. Commented Mar 1, 2011 at 9:27
  • The context doesn't change anything in VB.NET terms. The C# code assigns to a value and returns the value on the same line and that is not done in VB.NET. And unless you're very ingrained in the C-is concepts, you may not know that the code fragment (Region.allByLanguage[key] = GeoController.RegionsByCountryAndLanguage(country, language)) actually returns a value. Besides the VB.NET code is not verbose. It's as clear as daylight. Commented Mar 1, 2011 at 11:26

3 Answers 3

Reset to default
7

Do you actually need access to the list instance also? If not, why not just write it like this?

 Region.allByLanguage[key] = GeoController.RegionByCountry(country, language));

Your syntax seems strange for C# also, and I don't think many would write that code that way.

Also, it's no goal in itself to reduce the number of lines of code, if it makes the code less readable as you are trying to do in this case.

If you are assigning to both Region.allByLanguage[key] and list, I would prefer two lines of code if I ever were to read your code. Merging both assignments on one line of code seems forced at best, and might have the reader not realize that list is assigned as well as Region.allByLanguage[key].

Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

Call me crazy, but the C# design team must have thought it might be good sugar in some circumstances, or else they wouldn't have supported it, I don't think. (That's not to say all language design decisions are good; many are poor choices. This, however, I don't think, is a poor choice.)
Heck, let's get rid of ternary operators because those are hard too. Let's go shopping!, instead.
Not all constructs allowed by a language should necessarily be used, even if allowed. See my example of nesten shorthands-if's on your question above. It's legal, you can nest it 5 levels deep if you want, but no-one will be able to read it. I don't say that what you proposing here is unreadable, but it's less readable than it's option of having several lines instead.
No worries man, I'm behind you and advocate readability of terseness. Just don't think it's applicable here. Different opinions is all. :)
Difference of opinions is quite ok, often even preferable to get a discussion going. But I would work on my tone, if I were you ;)
1

As an alternative, unless your cache is massive or doing something heavyweight, I would just change the code to something like this:

Dim key = Region.CacheKey(country, language)

If not Region.allByLanguage.ContainsKey(key) Then
    Region.allByLanguage(key) = GeoController.RegionsByCountryAndLanguage(country, language)
End If

Return Region.allByLanguage(key)

This way you can avoid the need for single line assignment, and you still reduce the lines of code. You can refactor it back out if performance becomes a problem.

Improve this answer

Comments

0

In VB.NET, you cannot assign to a variable and assign that same variable to another variable.

In your code, you're assigning GeoController.RegionByCountry(country, language) to list and then assigning list to Region.allByLanguage[key]

In most cases, this is not a good thing to do as it makes your code more difficult to read. If you are going to use the list variable later in the subroutine, it is better to go the VB way; explicitly assign the list and then assign it to the region. If you're not going to use it later then just assign result of GeoController.RegionByCountry(country, language) straight away and skip the temporary assignment to the list variable.

If you insist, however on a 'one-liner', you can use this code I was forced to write when I attempted porting some C# code to VB.NET some time ago:

<Extension()> Public Function AssignAndReturn(Of T)(ByVal source As T, ByRef target As T) As T
    target = source
    Return target
End Function

You can use it like so:

Region.allByLanguage(key) = GeoController.RegionByCountry(country, language).AssignAndReturn(list)

It evaluates the GeoController.RegionByCountry(country, language), assigns it to the list and then returns the result; which is the list.

Improve this answer

2 Comments

Ingenious! I can't bring myself to upvote this, in case it catches on & I find myself maintaining stuff like this, but it is ingenious.
@MarkJ: Thanks for the compliment. I had to force myself from using that when I discovered it in C# [implicitly] and ported it to VB.NET [explicitly]. I must admit I use it in extreme cases though.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.