电梯管理系统

Question

这是我在九月社区挑战上的尝试，做一个电梯处理系统。我对OOP并不十分熟悉，所以即使您不熟悉Python，我也特别希望获得对脚本这一部分的反馈。我相当肯定，我曾经错误地处理过抽象和应该使用哪个方法的类，但这需要您检查。

基本上，我的ElevatorSystem是您使用的主要类。实例化它将在其中创建所有必要的Elevator对象。Elevator执行一些小功能和反馈，但我的假设是，在一个真正的系统中，它将处理单个电梯的实际控制，移动楼层和开门。大多数情况下，Elevator对象实际上应该只是用作move_elevator调用的参数，否则就应该单独使用。

ElevatorSystem是魔法发生的地方。当有人呼叫电梯时，它会根据楼层邻近程度来选择最好的电梯，然后是电梯是否在移动，或者至少是朝着一个有利的方向移动，最后取决于电梯的使用程度。我有一个基本的wear属性在适当的地方，以防止某些电梯被不成比例地调用。

基本用途如下：

>>> es = ElevatorSystem(40, 5)
>>> ele = es.call_elevator(12, -1)
#Note that the result of call_elevator is assigned to ele for later use
Elevator 0 moving
On floor 1...
On floor 2...
<etc.>
On floor 12...
Doors opening on elevator 0.
>>> es.move_elevator(ele, 9)
#Now that the users have boarded elevator 'ele', pass ele to move_elevator
Elevator 0 moving
On floor 11...
On floor 10...
On floor 9...
Doors opening on elevator 0.

很明显，如果不设置某种多线程，那么多台电梯就可以同时调用，但我对此一无所知，我不想在这一挑战面前咬紧牙关。另外，sleep的实现仅仅是为了模拟实际电梯移动的时间传递。但是，如果不应用多线程，还是没有多大的价值。

from time import sleep

class Elevator():

    def __init__(self, num, floor=0):
        """Takes current floor as a parameter."""

        self.floor = floor
        self.num = num
        self.wear = 0
        self.direction = None

    def __str__(self):
        return "Elevator {} on floor {}".format(self.num, self.floor)

    def __repr__(self):
        return "<Elevator object {} currently on floor {}>".format(
                                            self.num, self.floor)

    def open(self):
        print ("Doors opening on elevator {}.".format(self.num))

    def close(self):
        print ("Doors closing on elevator {}.".format(self.num))

    def move(self, system):
        """Takes an ElevatorSystem and moves one floor in self.direction."""

        system.floors[self.floor].remove(self)
        self.wear += 1
        self.floor += self.direction
        system.floors[self.floor].append(self)

        print("On floor {}...".format(self.floor))

    def deactivate(self, system, warning=True):
        """Takes Elevator System to remove self from."""

        system.floors[self.floor].remove(self)
        if warning:
            print ("Elevator {} needs servicing. It is now deactivated".
                                                            format(self))

    def repair(self, system=None):
        """Resets wear optionally takes system to reattach to.

        If the elevator was broken, pass in an ElevatorSystem that
        the elevator should reattach itself to.
        For normal maintenance, this isn't required."""

        self.wear = 0
        if system is not None:
            system.floors[self.floor].append(self)
            print ("Elevator {} has been repaired.".format(self))


class ElevatorSystem():
    """A system for managing a building's elevators.

    Takes floor and elevator count as parameters.
    init will create the necessary Elevator objects.
    Active elevators are held on floors and accessed there."""

    #Constants to determine how in need of repair elevators are
    WEAR_WARNING = 4000
    WEAR_THRESHOLD = 5000

    def __init__(self, floor_count=3, elevator_count=2):
        if floor_count < 2 or elevator_count < 2:
            raise (ValueError, "Invalid parameters"
                   "Floor and elevator counts should be greater than 1.")

        self.elevators = [Elevator(i) for i in range(elevator_count)]
        self.floors = [[] for _ in range(floor_count)]
        self.floors[0] = self.elevators[:]
        self.deactivated = []

    def __repr__(self):
        return ("<ElevatorSystem object containing {} floors and {} elevators>"
                            .format(len(self.floors), len(self.elevators)))

    def __str__(self):
        return ("Elevator System with {} elevators on floors "
                .format(len(self.elevators)) + 
                ', '.join(str(elevator.floor if elevator.floor else 'G')
                              for elevator in self.elevators))

    def validate_floor(self, floor):
        """Raises a ValueError if floor doesn't exist."""

        if floor >= len(self.floors):
            raise ValueError("Invalid floor {}, system only "
                             "has {} floors.".format(floor, len(self.floors)))

    def choose_elevator(self, elevators, direction):
        """Return elevator that's most suitable.

        Takes in direction to first filter out moving elevators.
        Alternatively filters out elevators moving the wrong direction.
        Lastly will take the least worn elevator as its choice."""

        # First try take static elevators
        chosen_elevators = [elevator for elevator in elevators if
                                elevator.direction is None]

        # Now get elevators travelling in the right direction
        if not chosen_elevators:
            [elevator for elevator in elevators if
                elevator.direction == direction]

        #If that fails too, just take what we've got
        if not chosen_elevators:
            chosen_elevators = elevators

        #Sort by wear and take the first ie. lowest value in the sorted list
        return sorted(chosen_elevators, key=lambda ele: ele.wear)[0]


    def call_elevator(self, floor, direction):
        """Chooses and returns an elevator after moving it to floor.

        Floor should be the floor the elevator is called to.
        Direction should indicate which way the user wants to travel.
        Valid values are 1 or -1."""

        self.validate_floor(floor)
        if direction not in (1, -1):
            raise ValueError("direction only accepts 1 or -1, not {}".
                                                        format(direction))

        #Check if there's any elevators on the current floor
        if self.floors[floor]:
            elevator = self.choose_elevator(self.floors[floor], direction)
            elevator.open()
            return elevator

        #Now search outward, one floor up and down at a time.
        #None pads the list if we run out of floors in one direction
        check_floors = map(None, range(floor + 1, len(self.floors)),
                                 range(floor - 1, -1, -1))
        for up, down in check_floors:
            if down is not None and self.floors[down]:
                elevator = self.choose_elevator(self.floors[down], direction)
                break
            if up is not None and self.floors[up]:
                elevator = self.choose_elevator(self.floors[up], direction)
                break
        else:
            raise Exception("Cannot find elevators.")

        self.move_elevator(elevator, floor)
        return elevator       

    def move_elevator(self, elevator, floor):
        """Send elevator to floor, moving one floor at a time."""

        self.validate_floor(floor)
        print("Elevator {} moving".format(elevator.num))

        move_up = elevator.floor < floor
        elevator.direction = 1 if move_up else -1

        # Build the range based on which direction we need to go
        # We do this because range won't work if floor < elevator.floor
        for _ in (range(elevator.floor, floor) if move_up
                            else range(elevator.floor, floor, -1)):
            elevator.move(self)
            sleep(0.5)

        elevator.open()
        elevator.direction = None

    def maintenance_check(self, warnings=True):
        """Update list of deactivated elevators

        Optionally warns about elevators that need maintenance now or soon.
        Pass False to suppress printing these warnings."""

        for floor in self.floors:
            for elevator in floor:
                wear = elevator.wear
                if wear > self.WEAR_THRESHOLD:
                    self.deactivated.append(elevator)
                    elevator.deactivate(self, warnings)
                elif warnings and wear > self.WEAR_WARNING:
                    print ("{} has wear of {} and should be serviced soon".
                                format(elevator, elevator.wear))

    def repair_elevators(self, maintenance=False):
        """Repair or optionally service broken and worn elevators.

        maintenance determines whether or not to repair any elevator with
        significant wear or just to repair deactivated ones."""

        for elevator in self.deactivated:
            elevator.repair(self)

        if maintenance:
            for floor in self.floors:
                for elevator in floor:
                    if elevator.wear > self.WEAR_WARNING:
                        elevator.repair()

Answer 1

我没有时间进行适当的回顾，但是下面是一些基于快速浏览代码的评论：

• 我不喜欢电梯和ElevatorSystem的内部设备混淆的方式。它应该是一个独立的类，所有管理系统的工作都应该用ElevatorSystem来处理。
• 理想情况下，__repr__()应该返回一个字符串，在传递给eval()时给出一个相等的对象；例如：>>> e=电梯(num=2，floor=0) >>> repr(e)‘电梯(2，0)’
• 您没有给出电梯构造函数docstring中的num参数的任何解释。像id_num这样的东西可以明确地表明电梯是独一无二的。
• ElevatorSystem的楼层边界似乎有点模糊。例如，这里我实例化了一个有三层楼的系统，但我至少可以访问五个不同的楼层：>>> es = ElevatorSystem(3，2) >>> es.call_elevator(2，1)电梯0在1楼移动.在2楼..。电梯门开了。<电梯对象0目前在楼层2> >>> es.call_elevator(-2，1)电梯0在1楼移动。在0楼..。在第一层.在第二层.电梯门开了。<电梯对象0目前在楼层-2>注意到，如果只有三层，而我在二楼，那么我不能要求上去-但在ElevatorSystem没有任何突出的差异。试图到达太低的楼层会返回一些令人困惑的输出，而试图到达太高的楼层的是一个ValueError：>>> es.call_elevator(-3，1 )电梯1门打开。<电梯对象1目前在楼层0> >>> es.call_elevator(3，1)上回溯(最近一次呼叫)：文件""，第1行，在文件"elevator.py“中，第126行，在call_elevator self.validate_floor(楼层)文件"elevator.py”中，第93行，在validate_floor中“有{}层”。.format(层，连(self.floors)) ValueError:无效的层3，系统只有3层。我认为最好定义明确的下界和上界，并检查一下我是否试图超越这些界限。一楼上下的楼层数通常不是对称的。
• 电梯上的open()和close()方法并不能跟踪门是否已经打开。这可能导致混乱的情况下，门可以打开两次：>>> es.call_elevator(1，-1)电梯2在第1层移动.电梯门打开。<电梯对象2目前在电梯1> >>> es.call_elevator(1，-1)层，电梯2上开门。<电梯对象2目前在楼层1>我建议在电梯类中添加一个doors_open属性，该属性跟踪门是否打开，并且只在适当时打印门打开/关闭的消息。
• 在Elevator.deactivate()方法中，您将使用self而不是self.num来识别电梯。
• choose_elevator的这一部分什么也不做:如果不是chosen_elevators：电梯电梯的elevator.direction ==方向，您可以过滤符合给定方向的电梯，但是这个结果会被丢弃。