C#•2y ago

❔ 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•2y 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•2y ago

New, not working method (that method should return Left colleciton in reversed order:

tippy•2y ago

you could use .Reverse() on the collection i suppose?

Ice_trooper•2y ago

Reverse will call another IEnumerable, it wouldn't be performent 😅

Thinker•2y ago

unrelated but wtf is that type name T_Road_Lanes_LaneSection_Lr_Lane

Anton•2y ago

it being an IEnumerable is already bad for perf make a custom iterator if you care about that

Ice_trooper•2y ago

External library type

Ice_trooper•2y ago

I ended with something like this. I don't know how I can simplify this

Ice_trooper•2y ago

but Reverse built-in method would call another unnecessary IEnumerable 😅

Anton•2y 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•2y ago

You're using enumerables in the first place. You can't possibly be concerned about perf

Anton•2y 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•2y ago

And what you have there sure can be simplified a good bit

tippy•2y ago

dude, com on...

Ice_trooper•2y 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•2y 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();
    }
}

tippy•2y 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•2y 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

tippy•2y ago

seems reasonable and collection size?

Ice_trooper•2y 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.

tippy•2y 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•2y ago

it's hard to make any suggestions without more context

Ice_trooper•2y 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•2y 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?

tippy•2y ago

too slow

ero•2y ago

but i don't see how

MODiX•2y ago

Ice_trooper#2729

Quoted by

<@!654411275094327296> from #Return IEnumerable backwards (click here)

React with ❌ to remove this embed.

ero•2y 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•2y ago

yield return forces an allocation anyways

Accord•2y 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.

Gaming

Programming

❔ Return IEnumerable backwards