Stephan van Hulst wrote:The strategy would be used by the controller like this:
Maybe it's just me but in an interview setting, say with a time limit of a couple of hours, I wouldn't be looking for that much as an evaluator. In fact, if you came to me with that much "design," I'd be scared. I'd have to ask myself what kind of complex monstrosities would everybody have to deal with if we hired someone who could come up with all that in just a couple of hours? But again, it's just me. To others, having all that might be impressive but for me, simple is better.
For a first iteration, I would go with this design:
The strategy would be used by the controller like this:
Here is a Floor class you could use:
I would use a strong type to represent a floor. It will make your code much nicer to read, and you can also use it to represent levels that aren't associated with a number. Just this morning I stepped in an elevator that had the levels -1, E, L, 1, 2, 3, ...
I think the RequestDirection enum is useless. Instead of telling an elevator to move in a specified direction, just let it move closer to the next scheduled stop.
Why does ElevatorRequest have private setters? If the properties are intended to be read-only, remove the setters completely. Same goes for all your other classes.
The RequestTime parameter of the ElevatorRequest doesn't follow naming conventions. Start it with a lower-case letter.
What is the purpose of a Button? It looks like a useless wrapper around a floor ID. And what is the purpose of a ButtonPanel? Why not just call the ElevatorRequest constructor directly?
Why does an elevator need an ID?
The name 'capacity' does not clearly indicate whether it concerns weight or volume.
Property names should start with a capital letter.
I wouldn't use a PriorityQueue to hold your scheduled stops, because you will never be able to schedule stops in the opposite direction until the queue is completely empty. A simple List will do. Besides, I don't think PriorityQueue is a standard class in the .NET API.
Don't let a class react to events that it raises itself. Just call a method directly from the place where the event is raised.
SetElevatorRequest is a really poor name. Name it something like RequestRide instead.
Or is it that the strategy is used to decide whether or not to add a floor to an elevator's queue?
C# has properties. Why aren't you using them? Why aren't there event declarations in your API? Most importantly, why aren't there constructors, so I can see what dependencies a class relies on?
You're using inappropriate types. A long is a poor representation for an instant of time. Why do you have int to represent floors in some cases and char to represent them in other cases? Be consistent.
If you're using a Queue for stops, how are you going to insert a stop when it turns out that inserting a stop is more efficient than adding the stop at the end of the queue?
How is a scheduling strategy going to be effective when the only information it has is the request? Why doesn't a strategy have access to the available elevators or their current schedules?
What does it mean to "Set a request" in your ElevatorController?