4
\$\begingroup\$

Here's what I have at the moment. Any ideas for improving on this?

public static Snapshot[] GetSnapshots()
{
    var arrayedSnapshot = new
    {
        Id = Enumerable.Range(1, 9),
        ScenarioTimeInSeconds = new[] { 0, 0, 0, 60, 60, 60, 120, 120, 120 },
        BearingInDegrees = new[] { 0, 120, 220, 0, 120, 220, 0, 120, 220},
        RangeInNauticalMiles = new[] { 0, 1, 1, 0, 1, 1, 0, 1, 1 },
        CourseInDegrees = new[] { 90, 90, 90, 90, 90, 180, 90, 90, 90 },
        SpeedInKnots = Enumerable.Repeat(7, 9),
        XCoordinate = new[] { 0, 0, 0, 0, 0, 0, 0, 0, 0 },
        YCoordinate = new[] { 0, 0, 0, 0, 0, 0, 0, 0, 0 }
    };
    return arrayedSnapshot.Id
        .Select((td, index) =>
            new Snapshot
            {
                Id = arrayedSnapshot.Id ElementAt(index),
                ScenarioTimeInSeconds = arrayedSnapshot.ScenarioTimeInSeconds.ElementAt(index),
                BearingInDegrees = arrayedSnapshot.BearingInDegrees.ElementAt(index),
                RangeInNauticalMiles = arrayedSnapshot.RangeInNauticalMiles.ElementAt(index),
                CourseInDegrees = arrayedSnapshot.CourseInDegrees.ElementAt(index),
                SpeedInKnots = arrayedSnapshot.SpeedInKnots.ElementAt(index),
                XCoordinate = arrayedSnapshot.XCoordinate.ElementAt(index),
                YCoordinate = arrayedSnapshot.YCoordinate.ElementAt(index),
            })
        .ToArray();
}

Edit

Here is a new version based on Anton Golov's answer:

public static IEnumerable<Snapshot> GetSnapshots()
{
    for (var i = 0; i < 9; i++)
    {
        yield return new Snapshot
        {
            Id = i + 1,
            ScenarioTimeInSeconds = new[] { 0, 0, 0, 60, 60, 60, 120, 120, 120 }[i],
            BearingInDegrees = new[] { 0, 120, 220, 0, 120, 220, 0, 120, 220 }[i],
            RangeInNauticalMiles = new[] { 0, 1, 1, 0, 1, 1, 0, 1, 1 }[i],
            CourseInDegrees = new[] { 90, 90, 90, 90, 90, 180, 90, 90, 90 }[i],
            SpeedInKnots = new[] { 7, 7, 7, 7, 7, 7, 7, 7, 7 }[i],
            XCoordinate = new[] { 0, 0, 0, 0, 0, 0, 0, 0, 0 }[i],
            YCoordinate = new[] { 0, 0, 0, 0, 0, 0, 0, 0, 0 }[i]
        };
    }
}

This seems a lot more elegant than my original.

\$\endgroup\$
1
\$\begingroup\$

As far as I can see, most of your data is fairly predictable. What about:

for (int i = 0; i < 9; ++i)
{
   int scenarioTimeInSeconds = 60*(i/3); // Intentional loss of fractional part
   int bearingInDegrees = (i % 3) * 120; // It's now 0, 120, 240
   int rangeInNauticalMiles = (i % 3 == 0) ? 0 : 1;
   int courseInDegrees = (i == 5) ? 180 : 90;
   yield return new Snapshot
       {
           Id = i + 1,
           ScenarioTimeInSeconds = scenarioTimeInSeconds,
           BearingInDegrees = bearingInDegrees,
           RangeInNauticalMiles = rangeInNauticalMiles,
           CourseInDegrees = courseInDegrees,
           SpeedInKnots = 7,
           XCoordinate = 0,
           YCoordinate = 0,
      };
}

This already seems more readable to me. Now you can split the logic that makes the values for the various local ints off into separate functions and document each one. In order to do it this way, you do have to have GetSnapshots return an IEnumerable<Snapshot>, but that makes sense anyway -- and if you really want them as an array, just call ToArray() on it outside the function (i.e. GetSnapshots().ToArray()).

Even if you'd rather not do that, you should still change numbers that are used multiple times (in your case: 60, 90, 120, 180, 220) into named constants, unless they're entirely unrelated.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ +1, thanks. Great idea using yield (although I think it needs to be yield return new Snapshot...). I think I still prefer typing the lists out, though, because it's helpful for me to see the actual numbers in order when writing tests. See my edit. \$\endgroup\$ – devuxer Feb 2 '12 at 2:23
  • \$\begingroup\$ @DanM: Indeed, too much Python for me. To be honest, I find your original version to be cleaner than the new one, as it's not entirely clear what's happening. I would use operator[] instead of ElementAt if possible (surely there's some way to keep those variables arrays?), but if you insist on keeping those lists, I'm not sure how much can be done about it. (You can still use yield with foreach over arrayedSnapshot.Id, though, which may improve matters a little -- that LINQ query is really ugly, in my opinion.) \$\endgroup\$ – Anton Golov Feb 2 '12 at 2:36
  • \$\begingroup\$ I see your point, but the advantages of less typing and being able to quickly see the progression of values for each property are pretty compelling to me. I think once you get the hang of how the indexing works, it wouldn't be too hard to decipher, but I don't know. Anyway, thanks for your help. \$\endgroup\$ – devuxer Feb 2 '12 at 3:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.