none
[UWP][VB]Refactoring Adds Unnecessary Converter Methods RRS feed

  • Question

  • I currently have the following VB.NET Function:
    Private Function GetPlayerWidth(pd As PlayerData, expandsection As Boolean()) As Double
    	Dim expandpassengers As Boolean = expandsection(0)
    	Dim expandroutes As Boolean = expandsection(1)
    	Dim expandtickets As Boolean = expandsection(2)
    	Return {224,
    		pd.Name.MeasureText(CType(Me.Resources("NameTextBlockStyle"), Style)).Width,
    		If(expandpassengers, 26 * pd.Passengers.Max() + 30 + 40, 0),
    		If(Not expandpassengers, 26 * Math.Min(pd.PassengerPlaces.Select(Function(place, index) If(place = -1, 0, (pd.Passengers)(index))).ToArray().Sum(), 8) + pd.PassengerPlaces.Count(Function(place) place <> -1) * 30 + If(pd.PassengerPoints.Sum() < 100, 40, 60), 0),
    		If(expandroutes, 0, $"{pd.RouteCounts.Select(Function(count, index) count * (index + 1)).Sum()} Trains, {pd.RouteCounts.Sum()} Routes".MeasureText(CType(Me.Resources("SettingsLabelTextBlockStyle"), Style)).Width + pd.RoutePoints.Sum().ToString().MeasureText(CType(Me.Resources("SettingsLabelTextBlockStyle"), Style)).Width),
    		If(expandtickets, pd.Tickets.Select(Function(t) t.Name).MaximumTextBlockWidth(CType(Me.Resources("SettingsLabelTextBlockStyle"), Style)) + 20 + pd.Tickets.Select(Function(t) If(t.IsCompleted(pd), t.Points.ToString(), $"-{t.Points}")).MaximumTextBlockWidth(CType(Me.Resources("SettingsLabelTextBlockStyle"), Style)), 0)
    		}.Max()
    End Function

    I would like to use the "Inline temporary variable" refactoring capability for the expandpassengers, expandroutes, and expandtickets variables (yes, I know it's a very simple refactoring that is just a couple copy & pastes, but it's always nice to use Visual Studio 2017's refactoring capabilities). However, when I do the refactoring for expandpassengers (and I'm guessing it probably will for the other two as well) I get the following:
    Private Function GetPlayerWidth(pd As PlayerData, expandsection As Boolean()) As Double
    	Dim expandroutes As Boolean = expandsection(1)
    	Dim expandtickets As Boolean = expandsection(2)
    	Return Enumerable.Max({224,
    		CType(pd.Name, String).MeasureText(CType(CType(Me.Resources(CObj("NameTextBlockStyle")), Style), Style)).Width,
    		If(CBool(expandsection(CInt(0))), 26 * CType(pd.Passengers, IEnumerable(Of Integer)).Max() + 30 + 40, 0),
    		If(Not CBool(expandsection(CInt(0))), 26 * Math.Min(Enumerable.Sum(Enumerable.ToArray(Enumerable.Select(Of Integer)(CType(pd.PassengerPlaces, IEnumerable(Of Integer)), CType(Function(place, index) If(place = -1, 0, (pd.Passengers)(CInt(index))), Func(Of Integer, Integer, Integer))))), CInt(8)) + CType(pd.PassengerPlaces, IEnumerable(Of Integer)).Count(CType(Function(place) place <> -1, Func(Of Integer, Boolean))) * 30 + If(CType(pd.PassengerPoints, IEnumerable(Of Integer)).Sum() < 100, 40, 60), 0),
    		If(expandroutes, 0, TextBlockMeasurementExtensions.MeasureText($"{Enumerable.Sum(Enumerable.Select(Of Integer)(CType(pd.RouteCounts, IEnumerable(Of Integer)), CType(Function(count, index) count * (index + 1), Func(Of Integer, Integer, Integer))))} Trains, {CType(pd.RouteCounts, IEnumerable(Of Integer)).Sum()} Routes", CType(CType(Me.Resources(CObj("SettingsLabelTextBlockStyle")), Style), Style)).Width + CType(CType(pd.RoutePoints, IEnumerable(Of Integer)).Sum().ToString(), String).MeasureText(CType(CType(Me.Resources(CObj("SettingsLabelTextBlockStyle")), Style), Style)).Width),
    		If(expandtickets, TextBlockMeasurementExtensions.MaximumTextBlockWidth(Enumerable.Select(Of String)(CType(pd.Tickets, IEnumerable(Of Ticket)), CType(Function(t) t.Name, Func(Of Ticket, String))), CType(CType(Me.Resources(CObj("SettingsLabelTextBlockStyle")), Style), Style)) + 20 + TextBlockMeasurementExtensions.MaximumTextBlockWidth(Enumerable.Select(Of String)(CType(pd.Tickets, IEnumerable(Of Ticket)), CType(Function(t) If(t.IsCompleted(CType(pd, PlayerData)), t.Points.ToString(), $"-{t.Points}"), Func(Of Ticket, String))), CType(CType(Me.Resources(CObj("SettingsLabelTextBlockStyle")), Style), Style)), 0)
    		})
    End Function

    Notice how it added lots of the Enumerable namespace and unnecessary converter functions (CType, CObj, CBool, CInt, etc.). For example, pd.Name is already a String, so why is it adding a CType to it? expandsection is declared as Boolean(), so why is it adding CBool to it? 0 is a literal of type Integer, so why is it changing it to CInt(0)? You can definitely see that the refactoring made the code very messy and added lots of unnecessary converters. I know there are ways to customize refactoring preferences, but of the ones I could find none of them seemed to be very relevant to these (and why would anybody ever want to refactor the literal Integer 0 to CInt(0)?) Am I missing something, or is this just a scenario where the refactoring is better done manually? Thanks.

    Nathan Sokalski njsokalski@hotmail.com http://www.nathansokalski.com/

    Saturday, September 30, 2017 6:48 PM

All replies

  • For non-UWP specific questions about the VB language and editors please post in the VB forums.
    Sunday, October 1, 2017 3:30 AM