0
\$\begingroup\$

I am new to C# and am trying to understand if there is a very simple way to rewrite this code.

List<Vertex<string>> vertices = new List<Vertex<string>>
                (
                    new Vertex<string>[]
                        {
                    new Vertex<string>("Los Angeles"),
                    new Vertex<string>("San Francisco"),
                    new Vertex<string>("Las Vegas"),
                    new Vertex<string>("Seattle"),
                    new Vertex<string>("Austin"),
                    new Vertex<string>("Portland")
                        }
                );

                // Establish edges; Ex. Los Angeles -> San Francisco, Las Vegas, Portland
                vertices[0].AddEdges(new List<Vertex<string>>(new Vertex<string>[]
                {
                vertices[1], vertices[2], vertices[5]
                }));

some thing like the below to make it easy to understand

List<Vertex<string>> vertices = new List<Vertex<string>>();
            var vrtx = new Vertex<string>[]
                {
                new Vertex<string>("Los Angeles"),
                new Vertex<string>("San Francisco"),
                new Vertex<string>("Las Vegas"),
                new Vertex<string>("Seattle"),
                new Vertex<string>("Austin"),
                new Vertex<string>("Portland")
            };
\$\endgroup\$
1
  • \$\begingroup\$ For one work on indent \$\endgroup\$
    – paparazzo
    Commented Sep 14, 2017 at 0:59

1 Answer 1

4
\$\begingroup\$

As for initialization of List you can use this approach:

var vertices = new[]
    {
        "Los Angeles",
        "San Francisco",
        "Las Vegas",
        "Seattle",
        "Austin"
        "Portland"
    }
    .Select(city => new Vertex<string>(city))
    .ToList();

vertices will be of the List<Vertex<string>> type and you don't need to repeat new Vertex<string>(...) multiple times. Read about LINQ since it is one of the greatest features of C#.


I suggest to change the signature of the AddEdges method to

AddEdges(params Vertex<string>[] vertices)

Now you can call AddEdges in this way:

vertices[0].AddEdges(vertices[1], vertices[2], vertices[5]);

Read about params keyword.

Or you can define the method like:

AddEdges(IEnumerable<Vertex<string>> vertices)

and call it like this:

vertices[0].AddEdges(new[] { vertices[1], vertices[2], vertices[5] });

In my opinion it's not a good style to describe relations between objects via populating some collections using magic indices. I need to look at vertices list to know what is the vertices[2]. The biggest problem with your approach is if I change order of vertices inside vertices list, the cities network will be broken.

As an option you can declare enum with your cities:

public enum City
{
    LosAngeles,
    SanFrancisco,
    LasVegas,
    Seattle,
    Austin,
    Portland
}

To describe relations between them use Dictionary:

private static readonly Dictionary<City, City[]> CitiesEdges =
    new Dictionary<City, City[]>
    {
        [City.LosAngeles] = new[]
        {
            City.SanFrancisco,
            City.LasVegas,
            City.Portland
        },
        // ... add other edges
    };

And finally we'll create map for cities names:

private static readonly Dictionary<City, string> CitiesNames =
    new Dictionary<City, string>
    {
        [City.LosAngeles] = "Los Angeles",
        // ... add other names
    };

Then building of the entire cities network will be:

var verticesMap = Enum.GetValues(typeof(City))
                      .Cast<City>()
                      .ToDictionary(c => c,
                                    c => new Vertex<string>(CitiesNames[c]));

foreach (var vertex in verticesMap)
{
    var edges = CitiesEdges[vertex.Key].Select(c => verticesMap[c]);
    vertex.Value.AddEdges(edges);
}

var vertices = verticesMap.Values.ToList();

The big advantage of this approach is you can forget about edges building code and operate only by dictionaries above. If you need to add new city in your network you just add one new constant to City enum, new entry to CitiesEdges dictionary and new entry to CitiesNames dictionary.

All relations between cities is super clear since you now manipulate by semantically named constants rather than magic indices. Also now you don't depend on order of vertices inside vertices list.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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