Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

In addition to @radarbobs answer, a few more technical points about your code (mainly related to your usage of Linq):

  1. flightAskingForLanding is obtained using FirstOrDefault, yet your code will crash when building checkSchedule as you unconditionally access the variable.

  2. In FlightTower.CanTakeOff(), you do this:

     var arrivingFlights = _schedule.Where(x => x.ArrivalTime == DateTime.Now);
     if (arrivingFlights.ToList().Count == 0)
     {
     ...
     }
    
  • It's not necessary to create a list to get the count, the Linq extensions define a Count() extension method on IEnumerable<T>.
  • Furthermore there is an overload for Count() which accepts a predicate so you can shorten this to: if (_schedule.Count(x => x.ArrivalTime == DateTime.Now) == 0)
  • But actually you are not really interested in the count. What you want is "check if there aren't any flight arriving now" so you should use Any() (which can break early if the sequence is not empty): if (!_schedule.Any(x => x.ArrivalTime == DateTime.Now))
  1. This _schedule.FirstOrDefault(x => x.DepartureTime == _schedule.Min(c => c.DepartureTime)) is an O(N2) operation (for every item in schedule find the minimum in schedule and compare). Unfortunately there is no nice "return object where a certain property has the minimum" built into Linq but there are some alternative options some alternative options (worst case: first find the minimum and store in a local variable and then search the object)
  2. Above two points also apply to CanLand() and CheckRunwayStatus.

In addition to @radarbobs answer, a few more technical points about your code (mainly related to your usage of Linq):

  1. flightAskingForLanding is obtained using FirstOrDefault, yet your code will crash when building checkSchedule as you unconditionally access the variable.

  2. In FlightTower.CanTakeOff(), you do this:

     var arrivingFlights = _schedule.Where(x => x.ArrivalTime == DateTime.Now);
     if (arrivingFlights.ToList().Count == 0)
     {
     ...
     }
    
  • It's not necessary to create a list to get the count, the Linq extensions define a Count() extension method on IEnumerable<T>.
  • Furthermore there is an overload for Count() which accepts a predicate so you can shorten this to: if (_schedule.Count(x => x.ArrivalTime == DateTime.Now) == 0)
  • But actually you are not really interested in the count. What you want is "check if there aren't any flight arriving now" so you should use Any() (which can break early if the sequence is not empty): if (!_schedule.Any(x => x.ArrivalTime == DateTime.Now))
  1. This _schedule.FirstOrDefault(x => x.DepartureTime == _schedule.Min(c => c.DepartureTime)) is an O(N2) operation (for every item in schedule find the minimum in schedule and compare). Unfortunately there is no nice "return object where a certain property has the minimum" built into Linq but there are some alternative options (worst case: first find the minimum and store in a local variable and then search the object)
  2. Above two points also apply to CanLand() and CheckRunwayStatus.

In addition to @radarbobs answer, a few more technical points about your code (mainly related to your usage of Linq):

  1. flightAskingForLanding is obtained using FirstOrDefault, yet your code will crash when building checkSchedule as you unconditionally access the variable.

  2. In FlightTower.CanTakeOff(), you do this:

     var arrivingFlights = _schedule.Where(x => x.ArrivalTime == DateTime.Now);
     if (arrivingFlights.ToList().Count == 0)
     {
     ...
     }
    
  • It's not necessary to create a list to get the count, the Linq extensions define a Count() extension method on IEnumerable<T>.
  • Furthermore there is an overload for Count() which accepts a predicate so you can shorten this to: if (_schedule.Count(x => x.ArrivalTime == DateTime.Now) == 0)
  • But actually you are not really interested in the count. What you want is "check if there aren't any flight arriving now" so you should use Any() (which can break early if the sequence is not empty): if (!_schedule.Any(x => x.ArrivalTime == DateTime.Now))
  1. This _schedule.FirstOrDefault(x => x.DepartureTime == _schedule.Min(c => c.DepartureTime)) is an O(N2) operation (for every item in schedule find the minimum in schedule and compare). Unfortunately there is no nice "return object where a certain property has the minimum" built into Linq but there are some alternative options (worst case: first find the minimum and store in a local variable and then search the object)
  2. Above two points also apply to CanLand() and CheckRunwayStatus.
added 10 characters in body
Source Link
Jamal
  • 35.2k
  • 13
  • 134
  • 238

In addition to @radarbobs answer, a few more technical points about your code (mainly related to your usage of Linq):

  1. flightAskingForLanding is obtained using FirstOrDefault yet your code will crash when building checkSchedule as you unconditionally access the variable.

    flightAskingForLanding is obtained using FirstOrDefault, yet your code will crash when building checkSchedule as you unconditionally access the variable.

  2. I FlightTower.CanTakeOff() you do this:

    In FlightTower.CanTakeOff(), you do this:

     var arrivingFlights = _schedule.Where(x => x.ArrivalTime == DateTime.Now);
     if (arrivingFlights.ToList().Count == 0)
     {
     ...
     }
    
 var arrivingFlights = _schedule.Where(x => x.ArrivalTime == DateTime.Now);
 if (arrivingFlights.ToList().Count == 0)
 {
 ...
 }
  • It's not necessary to create a list to get the count, the Linq extensions define a Count() extension method on IEnumerable<T>.
  • Furthermore there is an overload for Count() which accepts a predicate so you can shorten this to: if (_schedule.Count(x => x.ArrivalTime == DateTime.Now) == 0)
  • But actually you are not really interested in the count. What you want is "check if there aren't any flight arriving now" so you should use Any() (which can break early if the sequence is not empty): if (!_schedule.Any(x => x.ArrivalTime == DateTime.Now))
  1. This _schedule.FirstOrDefault(x => x.DepartureTime == _schedule.Min(c => c.DepartureTime)) is an O(N^2N2) operation (for every item in schedule find the minimum in schedule and compare). Unfortunately there is no nice "return object where a certain property has the minimum" built into Linq but there are some alternative options (worst case: first find the minimum and store in a local variable and then search the object)
  2. Above two points also apply to CanLand() and CheckRunwayStatus.

In addition to @radarbobs answer, a few more technical points about your code (mainly related to your usage of Linq):

  1. flightAskingForLanding is obtained using FirstOrDefault yet your code will crash when building checkSchedule as you unconditionally access the variable.
  2. I FlightTower.CanTakeOff() you do this:
 var arrivingFlights = _schedule.Where(x => x.ArrivalTime == DateTime.Now);
 if (arrivingFlights.ToList().Count == 0)
 {
 ...
 }
  • It's not necessary to create a list to get the count, the Linq extensions define a Count() extension method on IEnumerable<T>.
  • Furthermore there is an overload for Count() which accepts a predicate so you can shorten this to: if (_schedule.Count(x => x.ArrivalTime == DateTime.Now) == 0)
  • But actually you are not really interested in the count. What you want is "check if there aren't any flight arriving now" so you should use Any() (which can break early if the sequence is not empty): if (!_schedule.Any(x => x.ArrivalTime == DateTime.Now))
  1. This _schedule.FirstOrDefault(x => x.DepartureTime == _schedule.Min(c => c.DepartureTime)) is an O(N^2) operation (for every item in schedule find the minimum in schedule and compare). Unfortunately there is no nice "return object where a certain property has the minimum" built into Linq but there are some alternative options (worst case: first find the minimum and store in a local variable and then search the object)
  2. Above two points also apply to CanLand() and CheckRunwayStatus.

In addition to @radarbobs answer, a few more technical points about your code (mainly related to your usage of Linq):

  1. flightAskingForLanding is obtained using FirstOrDefault, yet your code will crash when building checkSchedule as you unconditionally access the variable.

  2. In FlightTower.CanTakeOff(), you do this:

     var arrivingFlights = _schedule.Where(x => x.ArrivalTime == DateTime.Now);
     if (arrivingFlights.ToList().Count == 0)
     {
     ...
     }
    
  • It's not necessary to create a list to get the count, the Linq extensions define a Count() extension method on IEnumerable<T>.
  • Furthermore there is an overload for Count() which accepts a predicate so you can shorten this to: if (_schedule.Count(x => x.ArrivalTime == DateTime.Now) == 0)
  • But actually you are not really interested in the count. What you want is "check if there aren't any flight arriving now" so you should use Any() (which can break early if the sequence is not empty): if (!_schedule.Any(x => x.ArrivalTime == DateTime.Now))
  1. This _schedule.FirstOrDefault(x => x.DepartureTime == _schedule.Min(c => c.DepartureTime)) is an O(N2) operation (for every item in schedule find the minimum in schedule and compare). Unfortunately there is no nice "return object where a certain property has the minimum" built into Linq but there are some alternative options (worst case: first find the minimum and store in a local variable and then search the object)
  2. Above two points also apply to CanLand() and CheckRunwayStatus.
Source Link
ChrisWue
  • 20.6k
  • 4
  • 42
  • 107

In addition to @radarbobs answer, a few more technical points about your code (mainly related to your usage of Linq):

  1. flightAskingForLanding is obtained using FirstOrDefault yet your code will crash when building checkSchedule as you unconditionally access the variable.
  2. I FlightTower.CanTakeOff() you do this:
 var arrivingFlights = _schedule.Where(x => x.ArrivalTime == DateTime.Now);
 if (arrivingFlights.ToList().Count == 0)
 {
 ...
 }
  • It's not necessary to create a list to get the count, the Linq extensions define a Count() extension method on IEnumerable<T>.
  • Furthermore there is an overload for Count() which accepts a predicate so you can shorten this to: if (_schedule.Count(x => x.ArrivalTime == DateTime.Now) == 0)
  • But actually you are not really interested in the count. What you want is "check if there aren't any flight arriving now" so you should use Any() (which can break early if the sequence is not empty): if (!_schedule.Any(x => x.ArrivalTime == DateTime.Now))
  1. This _schedule.FirstOrDefault(x => x.DepartureTime == _schedule.Min(c => c.DepartureTime)) is an O(N^2) operation (for every item in schedule find the minimum in schedule and compare). Unfortunately there is no nice "return object where a certain property has the minimum" built into Linq but there are some alternative options (worst case: first find the minimum and store in a local variable and then search the object)
  2. Above two points also apply to CanLand() and CheckRunwayStatus.
lang-cs

AltStyle によって変換されたページ (->オリジナル) /