Problem: Traffic Flow

Question

I have the following task:

The Shallow Alto city council has organized a special committee to review the traffic situation in the city. Despite the fact that there is nothing whatsoever to do in the city, citizens still write complaints about the amount of time it takes them to get from one unexciting location to the next. The committee has been asked to figure out how long it takes someone to travel down the streets of Shallow Alto given the current traffic light programming. To do this, they want you to write an algorithm which, given the speed a car travels down a road and the timing of the traffic lights, returns the amount of time it takes the car to travel down the street. The traffic lights will be in an int[], with each element representing the amount of time in seconds between signal changes. Initially, all traffic lights have just turned green, and are at the beginning of their cycle. The order of the traffic lights in the int[] will be the order in which a car encounters them. The speed you will be given is in meters per second. Assume that the car starts 150 meters before the first traffic light, that there are 150 meters between each traffic light, and that the car stops 150 meters after the last traffic light. Disregard all acceleration and deceleration; a car is either at its given speed or entirely stopped, and it takes no time to go from one state to the other. If a car reaches a traffic light just as it turns red, it immediately stops and waits for it to turn green again (at which point it starts moving immediately). If the light just turned green, there is no wait and the car drives on through. Return the time, in seconds, that it takes the car to travel the entire distance. Round down any fractional parts, (for example 55.5 becomes 55 and 44.9 becomes 44), but do not do this until returning.

This is my first draft:

public class Car
{
    public double Speed { get;set; }

}

public class Light
{        
    public int Number { get; set; }

    public double SwitchTime { get; set; }
    public bool IsGreen(double time)
    {
        double num = Math.Floor(time / SwitchTime);

            return num % 2 == 0;            
    }
    public double TimeToWait(double time)
    {
        if (!IsGreen(time))
        {
            return ((time % SwitchTime) < 0.01) ? SwitchTime : (time % SwitchTime);
        }
        else
        {
            return 0;
        }
    }

}

class Traffic
{
    public Car mycar;
    public IList<Light> LightList;
    public static double lengthbetweenLights { get; set; }

    public double getTravelTime()
    {
        double currenttime = lengthbetweenLights / mycar.Speed;            

        foreach (Light light in LightList)
        {               
            currenttime += lengthbetweenLights / mycar.Speed + light.TimeToWait(currenttime);                
        }

        return currenttime;
    }
}

class Program
{
    static void Main(string[] args)
    {
        var car1 = new Car
        {
            Speed = 20
        };
        List<Light> lights1 = getLightList();

        var mytraffic = new Traffic
        {
            mycar = car1,
            LightList = lights1

        };

        Traffic.lengthbetweenLights = 150; 

        var traveltime = mytraffic.getTravelTime();

        Console.WriteLine(string.Format("Time for car: {0}", traveltime));
        Console.ReadLine();  
    }

    private static List<Light> getLightList()
    {
        List<Light> lstProducts = new List<Light>();
        //input 1
        lstProducts.Add(new Light { Number = 0, SwitchTime = 10 });
        lstProducts.Add(new Light { Number = 1, SwitchTime = 20 });
        lstProducts.Add(new Light { Number = 2, SwitchTime = 30 });
        return lstProducts;
    }


}

You can also see my code on GitHub.

I need to improve my code, make it more OOP and use Design Patterns.

Question is: which design patterns I can use here?

Answer 1

Naming

I strongly recommend you read through the official C# naming conventions. Here are a few rules to keep in mind:

• Properties are UpperCamelCase
• Private fields are lowerCamelCase, sometimes prepended with an underscore
• Methods are UpperCamelCase
• Local variables are lowerCamelCase
• Test methods should be descriptive [more information]

Applied to your code, here are a few changes to make:

• TestMethod1 -> TimeToWait_WithFiveMinutes_ShouldReturnZero
• mycar -> currentCar
• lengthbetweenLights -> lengthBetweenLights
• getTravelTime -> GetTravelTime

Also keep in mind that your naming should make sense in light of the contest: MyCar does not really make sense in the sense of a Traffic analysis: what if I want to calculate it with someone else's car? Therefore I suggest making it CurrentCar instead.

This is part of the encapsulation I talk about next, but keep in mind that hiding the implementation details by not putting them in the variable's name, is also a form of encapsulation.

Right now you have a variable named LightList which tells me that it concerns a list of lights. Except, I already know it's a list because I can see the type. But what if you want to go more general and make it a ICollection instead? Or you want to group them with the amount of dogs that peed against it: Dictionary<Light, int>. Now your name doesn't fit anymore and you'll have to change it (and make version control ugly).

Methods are actions: you perform an action on the instance. What kind of action is TimeToWait? I would have expected it to be a property from the way it's worded. GetTimeToWait makes it clear that I will receive a value (and just that).

Encapsulation

Definitely read more on the topic of encapsulation, a cornerstone of OO-Programming. Your Traffic class has public fields which are open for modification. By forcing to do this through setters (an auto-generated property is a setter!) you can satisfy encapsulation and force whoever wants to modify it to go through your setter first. This means that you can add validation logic before touching the backing variable.

Instead what you should do is work with a public property like

public Car CurrentCar { get; set; };

[More information on fields and properties]

Events

Traffic lights that get their status updated every second sounds like a good option to put events in here. You could keep counters elapsedTime and switchTime and a flag isGreen so your light can articulate its state to callers. This would make your methods IsGreen() and GetTimeToWait() more intuitive: you don't have to call them with an argument to signify the current time.

Although this might be overdoing the assignment a little bit.