HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcsharpMinor

Airline seating algorithm: how to seat a passenger in mirror image to previous allocated seat

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
codereviewcsharpstackoverflowalgorithmc#
airlineimagepreviousmirrorseatingalgorithmallocatedhowpassengerseat

Problem

I have a seat allocation algorithm for an airplane 30 x 6 seats.
The plane is 'split' front (1 - 15) and back (16 -30), right (A,B,C) and left (D,E,F). I am placing passengers in window seats front right centre (15A) first then back left centre (16F) followed by (15F) followed by (16A). This will continue for single passengers middle seats then aisle.

Another part of the algorithm is to ensure parties >2 or <=6 to sit together in an empty row, if not possible the party will be split as to not have a person in the party sitting alone firstly. i.e. 6 is 3 + 3 or 2 + 2 + 2 or 4 + 2 or 3 + 2 + 1 or 4 + 1 + 1 or 3 + 1 + 1 + 1 and so.

I am wondering if anyone has any input to maybe refactor it, a different approach to allocating seats on an airline taking into consideration load and grouping parties or any other thoughts? I have been asked that if there is a manual selection of 1A - 1F will the alogrithm mirror this and place six passengers automatically to the back 30A -30F or next clear row. Is this possible? All comments welcome.

```
using ARS.Models;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Web;
using System.Web.Mvc;

namespace ARS.Controllers
{
public class DemoSeatController : Controller
{
const int seatCapacity = 180;
const int maxRow = 6;
const int maxCol = 30;

const int middleCols = maxCol / 2;
const int middleRows = maxRow / 2;

int countFrontPlane = 0, countBackPlane = 0, countLeftPlane = 0, countRightPlane = 0;

// GET: DemoSeat
public ActionResult DemoSeat()
{
ViewBag.PartySizeDDL = new SelectList(populatePartySizeDDL(), "Value", "Text", "1");
return View();
}

//Post: DemoSeat
[HttpPost]
public ActionResult DemoSeat(int party)
{
//create list of seats
List seats = new List { };
SeatSelectionController ssc = new SeatSelectionContr

Solution

You have business logic in the controller, which you shouldn't. There are people that may have said it OK. It's not.

I have extracted methods twice from you DemeSeat method. First removing dependency to the web frameworks, and a second time that removed dependency on model. That method may be tested, as it should, in isolation.

There is these horrible lines:

seatArray = seatSixPerson(seatArray, countFrontPlane <= countBackPlane ? middleCols - 1 : middleCols, countFrontPlane <= countBackPlane ? 0 : maxCol, countFrontPlane <= countBackPlane ? -1 : 1, countRightPlane <= countLeftPlane ? 0 : maxRow - 1, countRightPlane <= countLeftPlane ? maxRow : 0, countRightPlane <= countLeftPlane ? 1 : -1);


If a line is longer than 80 and it can be shortened, that it should be. If it a line is longer than 100-120 it definitely should. My editor says above line is 360+ long.

Above line is also mostly copy-pasted, several times. I had to use search&replace to make sure there wasn't any subtle differences between them.

Here party is a method parameter:

switch (party)
// .....

default:
    Console.WriteLine("Default case");
    break;


If 1..6 is the only valid party throw and ArgumentException for all others.

More than a few parameters are a code smell.
Here:
public bool[,] seatTwoPerson(bool[,] seatArray, int startCol, int endCol, int directionX, int startRow, int endRow, int directionY)

int startCol, int endCol, int directionX, int startRow, int endRow, int directionY appear together everywhere give that concept a name and use it: 

public class SearchBox
{
    public int startCol;
    public int endCol;
    public int directionX;

    public int startRow;
    public int endRow;
    public int directionY;
}


Your SeatNPerson methods both modify its first parameter, seatArray, and return it. A method should either modify something and return nothing, or vice versa. (Command/Query Separation). In this case they should declare void return type. By C# convention you can merge bool canSeatX and void SeatX, if there is significant gain to be had, with a method bool TrySeatX.

The main routine after some refactorings:

void SeatParty(ref List seats, int party)
{
    bool[,] seatArray = new bool[maxCol, maxRow];
    int i = 0;
    for (int row = 0; row  6) throw new ArgumentException("blah");

    if (party > freeSeats)
    {
        return false;
    }

    int startCol = countFrontPlane [] actions = {null, 
        seatOnePerson, seatTwoPerson,
        seatThreePerson, seatFourPerson,
        seatFivePerson, seatSixPerson};

    actions[party](seatArray, searchBox);

    return true;
}


SeatNPerson methods should be looked at; after there is a test harness, however basic, in place; and after some clean up as shown above is done. As they are too complicated to hold in one's mind, what they are doing and all they are doing and if they are doing it correct etc.

Code Snippets

seatArray = seatSixPerson(seatArray, countFrontPlane <= countBackPlane ? middleCols - 1 : middleCols, countFrontPlane <= countBackPlane ? 0 : maxCol, countFrontPlane <= countBackPlane ? -1 : 1, countRightPlane <= countLeftPlane ? 0 : maxRow - 1, countRightPlane <= countLeftPlane ? maxRow : 0, countRightPlane <= countLeftPlane ? 1 : -1);
switch (party)
// .....

default:
    Console.WriteLine("Default case");
    break;
public class SearchBox
{
    public int startCol;
    public int endCol;
    public int directionX;

    public int startRow;
    public int endRow;
    public int directionY;
}
void SeatParty(ref List<Seat> seats, int party)
{
    bool[,] seatArray = new bool[maxCol, maxRow];
    int i = 0;
    for (int row = 0; row < maxRow; row++) {
        for (int col = 0; col < maxCol; col++) {
            seatArray[col, row] = seats.ElementAt(i).IsReserved;
            if (seats.ElementAt(i).IsReserved) {
                if (col < middleCols) {
                    countFrontPlane++;
                } else {
                    countBackPlane++;
                }
                if (row < middleRows) {
                    countRightPlane++;
                } else {
                    countLeftPlane++;
                }
            }
            i++;
        }
    }

    int freeSeats = seatCapacity - (countFrontPlane + countBackPlane);

    SeatArray(seatArray, i, freeSeats, party);

    {
    i = 0;
    for (int row = 0; row < maxRow; row++) {
        for (int col = 0; col < maxCol; col++) {
            seats.ElementAt(i).IsReserved = seatArray[col, row];
            i++;
        }
    }
    }

}

bool SeatParty(bool[,] seatArray, int i, int freeSeats, int party)
{
    if (party < 1 || party > 6) throw new ArgumentException("blah");

    if (party > freeSeats)
    {
        return false;
    }

    int startCol = countFrontPlane <= countBackPlane ? middleCols - 1 : middleCols;
    int endCol = countFrontPlane <= countBackPlane ? 0 : maxCol; 
    int directionX = countFrontPlane <= countBackPlane ? -1 : 1;
    int startRow = countRightPlane <= countLeftPlane ? 0 : maxRow - 1;
    int endRow = countRightPlane <= countLeftPlane ? maxRow : 0;
    int directionY = countRightPlane <= countLeftPlane ? 1 : -1;

    var searchBox = new SearchBox
    {
        startCol = startCol,
        endCol = endCol,
        directionX = directionX,
        startRow = startRow,
        endRow = endRow,
        directionY = directionY,
    };


    Action<bool[,], Box>[] actions = {null, 
        seatOnePerson, seatTwoPerson,
        seatThreePerson, seatFourPerson,
        seatFivePerson, seatSixPerson};

    actions[party](seatArray, searchBox);

    return true;
}

Context

StackExchange Code Review Q#85562, answer score: 2

Revisions (0)

No revisions yet.