Cfrtjryk

The expected functionality is that a user/vehicle interacts with a ParkingLot and it allots the space in Park method based on type of vehicle (here I can expect a few changes, like increase from regular and handicapped to one more functionality for celebrities etc). What would be the route to perfect object oriented code?

class Vehicle

 attr_accessor :regno,:type

 def initialize(regno,type)

     @regno=regno

     @type=type

 end

end



class ParkingLot

 attr_accessor :size , :handicapped_size , :regular_size 



 def initialize(size)

    @lot={}

    @size=size

    @handicapped_size=(0.1)*size

    @regular_size=size-@handicapped_size



 end

 def check_regular

    @regular_size>0 ? @regular_size-=1 : false

 end

 def check_handicapped

    @handicapped_size>0 ? @handicapped_size-=1 : false

 end



 def park(vehicle,hour)

    case vehicle.type

    when "regular"

        check_regular ? @lot[vehicle.regno]=Regular_ParkingSpace.new(hour).payment : error()

    when "handicapped"

        check_handicapped ? @lot[vehicle.regno]=Handicapped_ParkingSpace.new(hour).payment : error()

    end

 end



 def unpark(vehicle)

    pay=@lot[vehicle.regno]

    puts "Pls pay us #{pay} rupees"

    @lot.delete(vehicle.regno){|el| puts "#{el} not found in this parking lot n"}

 end



 def error

    raise "No more vehicles can be parked !! n"

 end

end

class ParkingSpace

 attr_accessor :hour,:rate

 def initialize(hour,rate)

    @hour=hour

    @rate=rate

 end

 def payment

    @hour * @rate

 end

end



class Regular_ParkingSpace < ParkingSpace

 def initialize(hour,rate=20)

    super

 end

 def payment

    super

 end

end

class Handicapped_ParkingSpace < ParkingSpace

 def initialize(hour,rate=5)

    super

 end

end

Please mention the principle in use too.

Since @Flambino critiqued your existing code, I will focus more on application architecture.

When I approach this subject, first I determine what "things" are interacting in the system.

• Parking lots


• Vehicles


• Parking fees (of which there are "unrestricted" and "handicapped")


• Payments


• Parking spaces (of which there are "unrestricted" and "handicapped")

This gives us 5 classes. Classes are nouns (people, places or things) and methods are the verbs (actions). We will throw in two more classes to descibe the permits a vehicle can have (a handicapped sticker for those in the USA) and a custom error class for more intelligent error handling in the application.

1. Vehicle


2. ParkingFee


3. ParkingSpace


4. ParkingSpacePayment


5. ParkingLot


6. VehiclePermit


7. ParkingSpaceError

We'll get the easiest one of of the way:

class ParkingSpaceError < StandardError

end

Now we've got a specific error class when problems arise parking or paying for a parking spot allowing more intelligent error handling.

Once I've identified the basic "things" in the system, I think of how they are related. The first class I build is the one with no relations to anything. It's quick and simple: The ParkingFee class.

The ParkingFee class

A parking fee consists of a rate multiplied by how many hours you've parked. Plus, you only have two rates. We will create static properties for both fee types that we can reference later:

class ParkingFee

  def self.unrestricted

    @@unrestricted ||= ParkingFee.new 20

  end



  def self.handicapped

    @@handicapped ||= ParkingFee.new 5

  end



  def initialize(rate)

    @rate = rate;

  end



  def calculate(hours)

    rate * hours

  end



  def rate

    @rate

  end

end

Easy peasy, rice and cheesey. The next class that's easy to create is the VehiclePermit class.

The VehiclePermit class

Since you must determine whether or not a vehicle is a "handicapped vehicle," this got me thinking. First, I'm assuming since you reference "rupees" in your post that you are in India, or are building software that will be used in India. In the USA, vehicles can have multiple additional attributes describing how they can be used, and where they can be parked. I believe the term that we use is "permit."

Without understanding how vehicles are registered as "handicap" vehicles in India, I'll call this next class VehiclePermit. As we will see later, every Vehicle could have multiple permits attached to it. A "handicapped" vehicle is just a Vehicle that has the handicapped VehiclePermit associated with it.

This class doesn't have much to it. An id and a name, plus a static property for convenience. This could be just as easily mapped from a database if you want.

class VehiclePermit

  def self.handicapped

    @@handicapped ||= VehiclePermit.new 1, "Handicapped"

  end



  def initialize(id, name)

    @id = id;

    @name = name;

  end



  def id

    @id

  end



  def name

    @name

  end

end

Now we can move on to the "thing" this is all about: vehicles.

The Vehicle class

This class needs one or more permits, and a registration number. Still, not very complex. We'll also expose a public method to test this vehicle to see if it has the "handicapped" permit, which we will use later when parking the vehicle.

class Vehicle

  def initialize(registration_number)

    @registration_number = registration_number

    @permits = 

  end



  def is_handicapped?

    @permits.any? {|p| p == VehiclePermit.handicapped }

  end



  def permits

    @permits

  end



  def registration_number

    @registration_number

  end

end

We have our ParkingFee, VehiclePermit and Vehicle, now we are ready to park this thing.

The ParkingSpace class

The ParkingSpace class holds a vehicle, the date and time the vehicle parked, and the fee associated with it. It allows you to calculate the payment as well. We'll define public methods for all the actions you can perform on a parking space:

• Park a car


• Vacate the space


• Calculate the fee


• Test to see if a vehicle is currently occupying it


• Test to see if it contains a particular vehicle


• Test a vehicle to see if it can park here

The code:

class ParkingSpace

  def initialize(parking_lot, fee, number)

    @parking_lot = parking_lot

    @fee = fee

    @number = number

  end



  def payment

    ParkingSpacePayment.new self

  end



  def can_park?(vehicle)

    !occupied?

  end



  def contains?(vehicle)

    self.vehicle == vehicle

  end



  def number

    @number

  end



  def occupied?

    !vehicle.nil?

  end



  def park(vehicle)

    raise ParkingSpaceError "Cannot park vehicle #{vehicle.registration_number}" unless can_park? vehicle

    self.vehicle = vehicle

    date_occupied = DateTime.current

  end



  def vacate

    payment = nil

    date_occupied = nil

    vehicle = nil

  end



  def vehicle

    @vehicle

  end



private



  def payment=(new_payment)

    @payment = new_payment

  end



  def calculate_fee(date)

    fee.calculate(date)

  end



  def vehicle=(new_vehicle)

    @vehicle = new_vehicle

  end

end

We aren't done yet, because there are two kinds of parking spaces. We will define concrete classes for each:

class UnrestrictedParkingSpace < ParkingSpace

  def initialize(parking_lot, number)

    super(parking_lot, ParkingFee.unrestricted, number)

  end

end

An "unrestricted" parking space doesn't require much code. We just override the constructor to pass the proper ParkingFee object. The can_park? method on the parent class has the correct logic for this type of space, so we don't bother overriding it.

class HandicappedParkingSpace < ParkingSpace

  def initialize(parking_lot, number)

    super(parking_lot, ParkingFee.handicapped, number)

  end



  def can_park?(vehicle)

    super(vehicle) && vehicle.is_handicapped?

  end

end

The "handicapped" parking space overrides the constructor, which passes the handicapped fee, and overrides the can_park? method. The can_park? method first delegates to the method on the super class, and if that returns true, we have an additional test to see if the vehicle has a handicapped permit, so we also call Vehicle#can_park?.

Each type of parking space has the fee baked in so no one can make the "Unrestricted" parking space fee less that 20 Rupees. Now that we've got a parking spaces, we need some place to put them: the ParkingLot.

The ParkingLot class

A parking lot has a bunch of spaces, both unrestricted and handicapped. We also need to do the following things in the parking lot:

• Park a vehicle


• Exit the parking lot


• Calculate a payment for a vehicle


• Check to see if spaces are available

The constructor for the ParkingLot class takes two arguments: the number of unrestricted spaces and the number of handicapped spaces.

class ParkingLot

  def initialize(unrestricted_count, handicapped_count)

    count = 0

    @parking_spaces = 



    unrestricted_count.times do |n|

      @parking_spaces << UnrestrictedParkingSpace.new self, n

    end



    count = @parking_spaces.count



    handicapped_count.times do |n|

      @parking_spaces << HandicappedParkingSpace.new self, count + n

    end

  end



  def exit(vehicle, payment)

    parking_space = parking_space_for vehicle

    raise ParkingSpaceError "Balance not paid: #{payment.balance}" unless payment.paid?

    parking_space.vacate vehicle

    parking_space

  end



  def park(vehicle)

    index = @parking_spaces.index {|space| space.can_park? vehicle }

    raise ParkingSpaceError "No spaces available" if index < 0

    parking_space = @parking_spaces[index]

    parking_space.park vehicle

    parking_space

  end



  def payment_for(vehicle)

    parking_space_for(vehicle).payment

  end



  def spaces_available?(vehicle = nil)

    if vehicle.nil?

      @parking_spaces.any? {|space| !space.occupied? }

    else

      @parking_spaces.any? {|space| space.can_park? vehicle }

    end

  end



private



  def parking_space_for(vehicle)

    index = @parking_spaces.index {|space| space.contains? vehicle }

    raise ParkingSpaceError "Vehicle #{vehicle.registration_number} is not parked in this lot" if index < 0

    @parking_spaces[index]

  end

end

Lastly, let's pay for our spot using a ParkingSpacePayment

The ParkingSpacePayment class

This class contains all the logic for paying for a parking spot. It needs the parking space, vehicle and fee. All three things are provided by the ParkingSpace object, so we require this in the constructor.

class ParkingSpacePayment

  def initialize(parking_space)

    @parking_space = parking_space

    @payment_date = DateTime.current

    @total_hours = ((@payment_date - parking_space.date_occupied) / 1.hour).round

    @amount_due = @parking_space.calculate_fee @total_hours

    @amount_paid = 0

  end



  def amount_due

    @amount_due

  end



  def amount_paid

    @amount_paid

  end



  def balance

    @amount_due - @amount_paid

  end



  def paid?

    @amount_paid <= 0

  end



  def parking_space

    @parking_space

  end



  def pay(amount)

    @amount_paid += amount

    balance

  end



  def payment_date

    @payment_date

  end



  def total_hours

    @total_hours

  end

end

Using these classes to manage a parking lot

Now that we've got all the pieces built, let's see how we can park a car, and pay for it.

begin

  # Our parking lot as 10 unrestricted spaces and 5 handicapped spaces

  parking_lot = ParkingLot.new 10, 5



  # Create two vehicles, one of each type.

  unrestricted_vehicle = Vehicle.new "1"

  handicapped_vehicle = Vehicle.new "2"

  handicapped_vehicle.permits << VehiclePermit.handicapped



  # Park the cars

  parking_lot.park unrestricted_vehicle

  parking_lot.park handicapped_vehicle



  # Let's do some shopping.

  sleep 3.hours



  # Time to check out.

  unrestricted_payment = parking_lot.payment_for unrestricted_vehicle

  handicapped_payment parking_lot.payment_for handicapped_vehicle



  # 20 * 3 hours should be 60. Opps! This could be a problem later

  unrestricted_payment.pay 55



  # 5 * 3 hours = 15

  handicapped_payment.pay 15



  # I guess we went to the ATM. Whew!

  unrestricted_payment.pay 5 unless unrestricted_payment.paid?



  # Now let's exit the parking lot. Time to go home.

  unrestricted_space = parking_lot.exit unrestricted_vehicle, unrestricted_payment

  puts unrestricted_space.occupied? # -> "false"



  handicapped_space = parking_lot.exit handicapped_vehicle, handicapped_payment

  puts handicapped_space.occupied? # -> "false"

rescue ParkingSpaceError => parking_error

  puts "Oops! Had a problem parking a car: #{parking_error}"

else Exception => e

  raise e # A non parking error occurred.

end

Extra Credit

Since a parking lot could have multiple levels and rows on each level, the layout of a parking lot could be abstracted away to a ParkingLotLayout class, which takes a ParkingLot as a constructor argument and defines the floors and rows that are available.

1. Indentation and whitespace
   
   The Ruby convention is 2 spaces of indentation, and blank lines between methods. I'd also recommend spaces between arguments and operators, e.g. @hour = hour.




2. Naming
   
   Don't use underscores in class names. It's clear that Regular_ParkingSpace is a kind of ParkingSpace because it inherit directly from that parent class. A name like RegularSpace would be more straightforward and less of a mouthful.




3. attr_accessor
   
   You're adding a number of synthesized accessor methods with attr_accessor but you never use those methods. Instead you access instance variables directly. My advice is to always use accessor methods when you can. However, attr_accessor generates both readers and writers, and they're public. Which means that external code can just say parking_lot.size = 9999, which doesn't make sense. Of course, you never actually use the size attribute for anything, which brings me to my next point:




4. Junk code
   
   It sounds harsh, but I just mean "code that doesn't actually do anything". For example the @size variable in ParkingLot which is never used. Or Regular_ParkingSpace having a payment method that just calls super - something that'd happen automatically if the method wasn't there.




5. Dangerous assumptions
   
   Speaking of the parking lot's size, your way of determining the number of handicap spaces is not vary robust. You just assume it's going to be one tenth of the spaces. Well, what if the parking lot has 8 spaces in total? Then you have 0.8 of a handicap space, and 7.2 regular ones. Or what if it has 213 spaces? Then you have 21.3 handicap spaces. Neither situation makes any sense. For that matter who says there are any handicap spaces at all? There's no reason - that I know if - to assume there's any proportional relationship between the two numbers.




6. Outright bugs
   
   Following from the above: The way you check for remaining spaces assumes integers. If the remaining number of spaces is anything above zero, you assume that that means there's a whole parking space there. So, in turn, you assume that whatever size was originally passed to ParkingLot.new is cleanly divisible by 10. But that's not given.
   
   End result is that if I make a parking lot with 8 spaces, I can fit 9 vehicles: 1 handicap vehicle (in the 0.8 of a space), and 8 regular ones (the first 7 get a space each, and the last one has to fit in 0.2 of a space).
   
   Oh, and I can just park a car for zero hours, and I'll pay zero rupees.




7. Pointless classes
   
   You parking lot classes don't really serve any purpose being classes. You instantiate one of them, only to call payment and then discard the instance. In the end, you classes could be replaced with methods, or even just an expression: a * b.




8. Informal exceptions
   
   Don't just raise a string; create an exception class that inherits from StandardError and raise it instead.

In other words, there's a lot going on here. Making it "more object-oriented" is a secondary concern. And with no concept of time passing, hourly rates don't really make much difference. It's a little weird that the cost gets calculated immediately when parking, and "paid" when the car's retrieved. A real parking lot would do one or the other: Pre-pay for x amount of time (with the possibility of an extra fee if you overstay), or pay for time used when leaving. This is neither of those.