Problem: Traffic Flow

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; }

}

```
pub

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. 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.

StackExchange Code Review Q#50918, answer score: 10