Skip to content

Conversation

@Slavunderkind
Copy link
Owner

No description provided.

@Slavunderkind Slavunderkind requested review from a user and vasil-gochev January 11, 2017 14:26
@Slavunderkind Slavunderkind changed the title Fix type error - defibrillator instead of defibrilator [Defibrillators task] Fix type error - defibrillator instead of defibrilator Jan 12, 2017
@Slavunderkind Slavunderkind force-pushed the master branch 4 times, most recently from b8e5d88 to 73f6435 Compare January 17, 2017 14:56
@Slavunderkind Slavunderkind force-pushed the defibrillator_solution branch from 6a6daec to bbc0b34 Compare January 17, 2017 15:03
def init_defibrilators(defibrilator)
d_long = parse(defibrilator[4])
d_lat = parse(defibrilator[5])
def init_defibrillators(defibrillator)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the function is called init_defibrillators, it should take all the objects at once. If you're initializing one, it's better to make the method singular.

Also, add_defibrillator might be a better name, because you're adding it to the existing defibrillators hash

d_long = parse(defibrilator[4])
d_lat = parse(defibrilator[5])
def init_defibrillators(defibrillator)
d_long = parse(defibrillator[4])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parse is too generic of a name.

end

def start
n.times do
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here you have the option of either adding inputs one by one, or getting the whole input and parsing it in one go. The latter might be a better option

init_defibrilators(defib.split(';'))
init_defibrillators(defib.split(';'))
end
puts find_closest(defibrillators)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Ruby object itself should only return the answer. Printing should happen after the fact.
So instead of:
Object.start
You can do

answer = object.start
puts answer

def find_closest(defibrilators_hash)
min = defibrilators_hash.values.first
closest_name = defibrilators_hash.keys.first
def find_closest(defibrillators_hash)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of writing a method yourself, you can use Enumerable#min_by:

closest_name, closest_value = defibrillators_hash.min_by { |pair| pair.last }

Also, if you're not going to use the second value, you can write it like this:

closest_name, _ = defibrillators_hash.min_by { |pair| pair.last }

x = calculate_x(d_long, d_lat)
y = calculate_y(d_lat)
defibrilators[defibrilator[1]] = distance(x, y)
defibrillators[defibrillator[1]] = distance(x, y)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inside the distance function, you can use Ruby's "power" operator - so instead of

x * x

you can do

x ** 2


def start
n.times do
Array.new(defibrilators_count) do
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@evgenispasov-which special for you

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants