patternrubyMinor
Simple Ruby program to model a prepaid transit card system
Viewed 0 times
simpletransitsystemprogramcardprepaidrubymodel
Problem
I have written a simple Ruby program to model a transit card system which aims to address the following user stories. I wanted to see whether I could get any feedback on it. My main interest is understanding where I have made fundamental mistakes that violate OOP(SOLID) Principles with a real focus on Single Responsibility.
I have included the code and specs and link to my repo - https://github.com/benhawker/oyster-cards.
-
In order to use public transport
As a customer
I want money on my card
-
In order to keep using public transport
As a customer
I want to add money to my card
-
In order to protect my money
As a customer
I don't want to put too much money on my card
-
In order to pay for my journey
As a customer
I need my fare deducted from my card
-
In order to get through the barriers
As a customer
I need to touch in and out
-
In order to pay for my journey
As a customer
I need to have the minimum amount for a single journey
-
In order to pay for my journey
As a customer
I need to pay for my journey when it's complete
-
In order to pay for my journey
As a customer
I need to know where I've travelled from
-
In order to know how far I have travelled
As a customer
I want to know what zone a station is in
-
In order to be charged correctly
As a customer
I need a penalty charge deducted if I fail to touch in or out
-
In order to be charged the correct amount
As a customer
I need to have the correct fare calculated
```
require "./lib/journey"
class Oystercard
attr_reader :balance
MIN_BALANCE = 1
MAX_BALANCE = 50
def initialize(balance=0)
raise "Max balance is #{Oystercard::MAX_BALANCE}" if exceeds_max_balance?(balance)
@balance = balance
end
def top_up(amount)
raise "This would take you over the max balance!" if exceeds_max_balance?(@balance + amount)
@balance += amount
end
def tap_in(origin_station)
raise "Yo
I have included the code and specs and link to my repo - https://github.com/benhawker/oyster-cards.
-
In order to use public transport
As a customer
I want money on my card
-
In order to keep using public transport
As a customer
I want to add money to my card
-
In order to protect my money
As a customer
I don't want to put too much money on my card
-
In order to pay for my journey
As a customer
I need my fare deducted from my card
-
In order to get through the barriers
As a customer
I need to touch in and out
-
In order to pay for my journey
As a customer
I need to have the minimum amount for a single journey
-
In order to pay for my journey
As a customer
I need to pay for my journey when it's complete
-
In order to pay for my journey
As a customer
I need to know where I've travelled from
-
In order to know how far I have travelled
As a customer
I want to know what zone a station is in
-
In order to be charged correctly
As a customer
I need a penalty charge deducted if I fail to touch in or out
-
In order to be charged the correct amount
As a customer
I need to have the correct fare calculated
```
require "./lib/journey"
class Oystercard
attr_reader :balance
MIN_BALANCE = 1
MAX_BALANCE = 50
def initialize(balance=0)
raise "Max balance is #{Oystercard::MAX_BALANCE}" if exceeds_max_balance?(balance)
@balance = balance
end
def top_up(amount)
raise "This would take you over the max balance!" if exceeds_max_balance?(@balance + amount)
@balance += amount
end
def tap_in(origin_station)
raise "Yo
Solution
First,
There is no need to place calculation of
There is no need to call
Seems like you're not fully cover this part:
In order to be charged correctly
As a customer
I need a penalty charge deducted if I fail to touch in or out
When customer skips
If so,
deduct(Journey::PENALTY_FARE) in class Oystercard looks weird.There is no need to place calculation of
Journey's fare in Oystercard:class Journey
# def calculate_fare # replaced with #fare
# BASE_FARE + variable_trip_price
# end
def fare
if origin_station && destination_station
BASE_FARE + variable_trip_price
else
PENALTY_FARE
end
end
# rename #complete_journey >>> #complete_at
# @journey.complete_at(destination_station) is more readable
def complete_at(destination_station)
@destination_station = destination_station
end
private
#...
end
class Oystercard
def tap_out(destination_station)
@journey ||= Journey.new(nil)
@journey.complete_at(destination_station)
deduct(@journey.fare)
end
endThere is no need to call
true explicitly in below_min_balance? and exceeds_max_balance?:def below_min_balance?(balance)
balance MAX_BALANCE
endSeems like you're not fully cover this part:
In order to be charged correctly
As a customer
I need a penalty charge deducted if I fail to touch in or out
When customer skips
tap_out and tap_in again, previous @journey will be replaced with Journey.new, no charge applied.If so,
Oystercard::MIN_BALANCE > 5 - to prevent negative balance in that "edge-case".Code Snippets
class Journey
# def calculate_fare # replaced with #fare
# BASE_FARE + variable_trip_price
# end
def fare
if origin_station && destination_station
BASE_FARE + variable_trip_price
else
PENALTY_FARE
end
end
# rename #complete_journey >>> #complete_at
# @journey.complete_at(destination_station) is more readable
def complete_at(destination_station)
@destination_station = destination_station
end
private
#...
end
class Oystercard
def tap_out(destination_station)
@journey ||= Journey.new(nil)
@journey.complete_at(destination_station)
deduct(@journey.fare)
end
enddef below_min_balance?(balance)
balance < MIN_BALANCE
end
def exceeds_max_balance?(balance)
balance > MAX_BALANCE
endContext
StackExchange Code Review Q#118493, answer score: 2
Revisions (0)
No revisions yet.