C# lambda simplification

Question

I have a class player and player has a list of Shots. Each shot has its own yPos (position) because yPos of the player can change but shot will keep his position:

class Player
{
 public string Id { set; get; }
 public int yPos { set; get; }
 public List<Shot> shots;
 public Player(string _Id, int _yPos)
 {
 Id = _Id;
 yPos = _yPos;
 }
}
class Shot
{
 public int yPos { set; get; }
 public Shot(int _yPos)
 {
 yPos = _yPos;
 }
}

Then at some point in the game a have id, and I need to find player. and add to list of his shots a new shot with that's players position.
Here is what I ended up with:

string tempID = "xxx"; // not important where this temp id is coming from
players.Find(p => p.Id == tempID).shots.Add(new Shot(players.Find(p => p.Id == tempID).yPos));

And seems to be fine but looks supper strange. is there a way to simplify this statement so I wouldn't have to look up for the same player twice in one statement?

• mowwwalker: I don't understand Lambdas. They seem to sacrifice readability for inefficiency.
• Nikola Radosavljević: Its ok that you want a shortcut (I do this all the time), but wouldn't it make most sense to add Player.SetCurrentShotLoc() method? As Walkerneo pointed out, they are not very readable and you have to know very well when to use them. Use them in appropriate querying situations, not for iterations.
• NPSF3000: @Walkerneo Lambda's, when used appropriately, offer readability, performance and functionality.
• tsells: I used to not like lambda's a long time ago. It was because I didn't understand them. :) I can't live without them now.......

Answer 1

I would at least cache your player result:

var player = players.Find(p => p.Id == tempId);
player.shots.Add(new Shot(player.yPos));

• Jonas: Alternately, add a "AddShot" method to your Player class: AddShot() { this.shots.Add(new Shot(this.ypos)); }
• Eric King: I just suggested the same thing (re: AddShot). Great minds think alike. :-)
• Jonas: I'll take those complements from people w/ more rep than me :D

Answer 2

Instead of reaching into the Player to pull out its yPos value and creating a new Shot with it, and then shoving that Shot into the Player's Shot collection (how rude!), you could simplify things a bit by giving your Player a bit more smarts:

class Player
{
 public string Id { set; get; }
 public int yPos { set; get; }
 public List<Shot> shots;
 public Player(string _Id, int _yPos)
 {
 Id = _Id;
 yPos = _yPos;
 }
 public void AddShotToYPos()
 {
 shots.Add(new Shot(yPos));
 }
}

Then you could say:

players.Find(p => p.Id == tempID).AddShotToYPos();

• Chris Shain: +1 I much prefer this method, or if you don't want the behavior in the Player class, an extension method.

Answer 3

At the risk of possibly stating something obvious, this would do one less lookup, and is possibly more readable:

string tempID = "xxx";
var player = players.Find(p => p.Id == tempID);
player.shots.Add(new Shot(player.yPos));

Answer 4

KISS:

var p = players.Find(p => p.Id == tempID);
p.shots.Add(new Shot(p.yPos));

Pointlessly long LINQ:

(from p in players
 where p.Id == tempID
 select p).Take(1).ToList().ForEach(p => p.shots.Add(new Shot(p.yPos)));

Nice and Short Extension:

players.Find(p => p.Id == tempID).Shoot();
...
static class Extensions
{
 public static void Shoot(this Player p)
 {
 p.shots.Add(new Shot(p.yPos));
 }
}

Cooler Lambda

(from p in players
 where p.Id == tempID
 let x = new Func<bool>(() => { p.shots.Add(new Shot(p.yPos)); return true; })
 select x()).First();

• Guvante: I wouldn't consider caching KISS, but your code is an improvement.
• NPSF3000: @Guvante Given my alternatives [now added] - especially the LINQ solution - I think it is KISS :P