❔ Return IEnumerable backwards

Hey! I got a method which returns a specific collection depending of the parameter. Right now, I needed to add a method to return similar result, but Left.Lane collection should be returned in backward order. How can I do it in the more compact way (and without errors)?
30 Replies
Ice_trooper
Ice_trooperOP2y ago
Old, working method:
public IEnumerable<T_Road_Lanes_LaneSection_Lr_Lane> GetLanes (LaneSide side)
{
return side switch
{
LaneSide.Left => Left != null ? Left.Lane : Enumerable.Empty<T_Road_Lanes_LaneSection_Lr_Lane>(),
LaneSide.Right => Right != null ? Right.Lane : Enumerable.Empty<T_Road_Lanes_LaneSection_Lr_Lane>(),
LaneSide.Center => Center.Lane,
_ => throw new ArgumentOutOfRangeException(nameof(side), side, "This lane side is not a valid value.")
};
}
public IEnumerable<T_Road_Lanes_LaneSection_Lr_Lane> GetLanes (LaneSide side)
{
return side switch
{
LaneSide.Left => Left != null ? Left.Lane : Enumerable.Empty<T_Road_Lanes_LaneSection_Lr_Lane>(),
LaneSide.Right => Right != null ? Right.Lane : Enumerable.Empty<T_Road_Lanes_LaneSection_Lr_Lane>(),
LaneSide.Center => Center.Lane,
_ => throw new ArgumentOutOfRangeException(nameof(side), side, "This lane side is not a valid value.")
};
}
Ice_trooper
Ice_trooperOP2y ago
New, not working method (that method should return Left colleciton in reversed order:
Omnissiah
Omnissiah2y ago
you could use .Reverse() on the collection i suppose?
Ice_trooper
Ice_trooperOP2y ago
Reverse will call another IEnumerable, it wouldn't be performent 😅
Thinker
Thinker2y ago
unrelated but wtf is that type name T_Road_Lanes_LaneSection_Lr_Lane
Anton
Anton2y ago
it being an IEnumerable is already bad for perf make a custom iterator if you care about that
Ice_trooper
Ice_trooperOP2y ago
External library type
Ice_trooper
Ice_trooperOP2y ago
I ended with something like this. I don't know how I can simplify this
Ice_trooper
Ice_trooperOP2y ago
but Reverse built-in method would call another unnecessary IEnumerable 😅
Anton
Anton2y ago
if you make a custom iterator type it won't have to you can make the iterator reversed and also be a value type to avoid allocations if you're just going to foreach over it, it will be better
ero
ero2y ago
You're using enumerables in the first place. You can't possibly be concerned about perf
Anton
Anton2y ago
yeah the functional features in c# are implemented in a way that's quite bad for both perf and memory it's intentionally made more dynamic
ero
ero2y ago
And what you have there sure can be simplified a good bit
Omnissiah
Omnissiah2y ago
dude, com on...
Ice_trooper
Ice_trooperOP2y ago
I'm working in game dev, so yeah I'm not joking about performance ;p If I have a list, that is, an indexed collection then I don't want to unnecessarily call an additional IEnumerable to Reverse if I can do it in a more efficient way. I just want to write this in the more compact way. BTW I changed the above code to yield break instead of yield return null because of the error.
ero
ero2y ago
yeah again, if you actually did care about perf, you wouldn't use enumerables at all foreaching over an enumerable is incredibly costly something as simple as
IEnumerable<int> ints = new int[1];
foreach (int i in ints)
{
Console.WriteLine(i);
}
IEnumerable<int> ints = new int[1];
foreach (int i in ints)
{
Console.WriteLine(i);
}
becomes
IEnumerator<int> enumerator = ((IEnumerable<int>)new int[1]).GetEnumerator();
try
{
while (enumerator.MoveNext())
{
Console.WriteLine(enumerator.Current);
}
}
finally
{
if (enumerator != null)
{
enumerator.Dispose();
}
}
IEnumerator<int> enumerator = ((IEnumerable<int>)new int[1]).GetEnumerator();
try
{
while (enumerator.MoveNext())
{
Console.WriteLine(enumerator.Current);
}
}
finally
{
if (enumerator != null)
{
enumerator.Dispose();
}
}
Omnissiah
Omnissiah2y ago
you're even throwing exceptions... how many times would this code be called per second? also, have you tested performance at least?
Ice_trooper
Ice_trooperOP2y ago
You're right that foreach is more costly than for, but I wouldn't say that so much. Anyway, I've used them here to use the deferred execution functionality 😅 This exeception... should be exception. I've made it there only to throw something if someone would add new LaneSide (which should not happen) and it wouldn't be supported here. This method could be called at least once per frame
Omnissiah
Omnissiah2y ago
seems reasonable and collection size?
Ice_trooper
Ice_trooperOP2y ago
Reverse would make new allocation for a stack, then loop once over collection, and then again to use it. That's why I'm so reluctant to use the Reverse method When I code it myself it loops over collection only once and without any allocations. In that case the code is just quite long which I prefer to simplify :f Quite small (in the worst case max 16) So yeah, IEnumerable for small collection is not great. Still want to use deferred execution.
Omnissiah
Omnissiah2y ago
to me spending time optimizing this seems overkill as of now the only way i personally could think of simplifying it would be going unsafe
ero
ero2y ago
it's hard to make any suggestions without more context
Ice_trooper
Ice_trooperOP2y ago
You're probably right. And I don't want to use unsafe hah. Last question, could this code above be more readable for other programmers? Should I rewrite something?
ero
ero2y ago
public IEnumerable<T_Road_Lanes_LaneSection_Lr_Lane> GetLanesAwayFromCenterOrder(LaneSide side)
{
return side switch
{
LaneSide.Left => Left?.Lane.Reverse<T_Road_Lanes_LaneSection_Lr_Lane>() ?? Enumerable.Empty<T_Road_Lanes_LaneSection_Lr_Lane>(),
LaneSide.Right => Right?.Lane.Reverse<T_Road_Lanes_LaneSection_Lr_Lane>() ?? Enumerable.Empty<T_Road_Lanes_LaneSection_Lr_Lane>(),
LineSide.Center => Enumerable.Repeat(Center.Lane[0], 1),
_ => // ...
};
}
public IEnumerable<T_Road_Lanes_LaneSection_Lr_Lane> GetLanesAwayFromCenterOrder(LaneSide side)
{
return side switch
{
LaneSide.Left => Left?.Lane.Reverse<T_Road_Lanes_LaneSection_Lr_Lane>() ?? Enumerable.Empty<T_Road_Lanes_LaneSection_Lr_Lane>(),
LaneSide.Right => Right?.Lane.Reverse<T_Road_Lanes_LaneSection_Lr_Lane>() ?? Enumerable.Empty<T_Road_Lanes_LaneSection_Lr_Lane>(),
LineSide.Center => Enumerable.Repeat(Center.Lane[0], 1),
_ => // ...
};
}
what was the issue with this?
Omnissiah
Omnissiah2y ago
too slow
ero
ero2y ago
but i don't see how
MODiX
MODiX2y ago
Ice_trooper#2729
Reverse would make new allocation for a stack, then loop once over collection, and then again to use it. That's why I'm so reluctant to use the Reverse method When I code it myself it loops over collection only once and without any allocations. In that case the code is just quite long which I prefer to simplify :f
Quoted by
<@!654411275094327296> from #Return IEnumerable backwards (click here)
React with ❌ to remove this embed.
ero
ero2y ago
i don't understand i mean either way, you could
private IEnumerable<T...> LeftReverse
{
get
{
if (Left is null)
yield break;

// forr
}
}
private IEnumerable<T...> LeftReverse
{
get
{
if (Left is null)
yield break;

// forr
}
}
Akseli
Akseli2y ago
yield return forces an allocation anyways
Accord
Accord2y ago
Was this issue resolved? If so, run /close - otherwise I will mark this as stale and this post will be archived until there is new activity.
Want results from more Discord servers?
Add your server