none
Linq expression RRS feed

  • Question

  • I have this code and it works well. Just wondering if there is a neater way of writing the if statement nested in the foreach 


                var validateBooking = from customer in booking
                                      where customer.Status == false
                                      select customer;

                foreach (var validate in validateBooking) {
                    foreach(var flight in FlightCreator.StoredFlights.Flight) {
                        if ((validate.NumberOfTicket + flight.Booked) <= flight.Capacity) {
                            validate.Status = true;
                            flight.Booked = validate.NumberOfTicket + flight.Booked;
                        }
                    }
                }


    Tony Greenleaf

    Monday, April 2, 2018 12:30 PM

Answers

  • Looks good.

    What you could do is write various unit test methods against the code shown were you cover both perfect outcomes and ones where there is invalid or bad data (null elements etc) pushed to the code shown. 

    In regards to neater, neater is not always better :-)


    Please remember to mark the replies as answers if they help and unmark them if they provide no help, this will help others who are looking for solutions to the same or similar problem. Contact via my Twitter (Karen Payne) or Facebook (Karen Payne) via my MSDN profile but will not answer coding question on either.
    VB Forums - moderator
    profile for Karen Payne on Stack Exchange, a network of free, community-driven Q&A sites

    Monday, April 2, 2018 1:06 PM
    Moderator
  • You can use LINQ to do the enumerations but you are modifying data within the enumerations and that isn't what LINQ is for. You'll still end up with some foreach statements to set the values. Of course you could wrap that in an extension method to hide it but you're still going to have one.

    As Karen mentioned, if it is working I'd probably leave it alone. I can see where you could possibly rewrite this code to make it better but that would be highly dependent upon how the data is laid out and would probably make it longer. In your code you're enumerating through once and both setting the status of the booking and updating the flight booked status. This may get confusing if you have to add more stuff to do in later versions of the app.


    Michael Taylor http://www.michaeltaylorp3.net

    Monday, April 2, 2018 2:08 PM
    Moderator

All replies

  • Looks good.

    What you could do is write various unit test methods against the code shown were you cover both perfect outcomes and ones where there is invalid or bad data (null elements etc) pushed to the code shown. 

    In regards to neater, neater is not always better :-)


    Please remember to mark the replies as answers if they help and unmark them if they provide no help, this will help others who are looking for solutions to the same or similar problem. Contact via my Twitter (Karen Payne) or Facebook (Karen Payne) via my MSDN profile but will not answer coding question on either.
    VB Forums - moderator
    profile for Karen Payne on Stack Exchange, a network of free, community-driven Q&A sites

    Monday, April 2, 2018 1:06 PM
    Moderator
  • You can use LINQ to do the enumerations but you are modifying data within the enumerations and that isn't what LINQ is for. You'll still end up with some foreach statements to set the values. Of course you could wrap that in an extension method to hide it but you're still going to have one.

    As Karen mentioned, if it is working I'd probably leave it alone. I can see where you could possibly rewrite this code to make it better but that would be highly dependent upon how the data is laid out and would probably make it longer. In your code you're enumerating through once and both setting the status of the booking and updating the flight booked status. This may get confusing if you have to add more stuff to do in later versions of the app.


    Michael Taylor http://www.michaeltaylorp3.net

    Monday, April 2, 2018 2:08 PM
    Moderator
  • You could use the full version of Resharper free usage for 30 days, point it at the code you are posting about, see if Resharper recommends coding changes, and let it make the changes, which can be undone if you don't like the changes

    I could see Resharper could make some changes possibly.

    https://stackoverflow.com/questions/1336177/replacing-nested-foreach-with-linq-modify-and-update-a-property-deep-within

     
    Tuesday, April 3, 2018 12:09 AM
  • I have this code and it works well. Just wondering if there is a neater way of writing the if statement nested in the foreach 


                var validateBooking = from customer in booking
                                      where customer.Status == false
                                      select customer;

                foreach (var validate in validateBooking) {
                    foreach(var flight in FlightCreator.StoredFlights.Flight) {
                        if ((validate.NumberOfTicket + flight.Booked) <= flight.Capacity) {
                            validate.Status = true;
                            flight.Booked = validate.NumberOfTicket + flight.Booked;
                        }
                    }
                }


    Tony Greenleaf

    This can certainly be improved and the loop made smaller.

    It's very hard though without actual compilable source code!;

    This kind of thing springs to mind but as I say this isn't valid because I have no details:

    var validateBooking = booking.customer.Where(c => c.Status == false);
    
    
    var matches = validateBooking.Select(v => new {v, FlightCreator.StoredFlights.Flight.Where(f => (v.NumberOfTicket + f.Booked) <= f.Capacity)}).SelectMany();
    
    // loop here

    if this was done correctly you'd be able to just loop through items in "matches" and modify each on as you needed to.



    Sunday, May 6, 2018 9:46 PM