Return to Answer

added 972 characters in body

Source Link

edited Jun 3, 2015 at 18:38

1.1k
6
16

def full_name
    full_name =info[:first_name]
if !@middle_name.nil?
    full_name += " "
    full_name += info[:middle_name]
end
    full_name += ' '
    full_name += info[:last_name]
    full_name
end

Your indentation is messed up here, this should be

def full_name
  full_name =info[:first_name]
  if !@middle_name.nil?
    full_name += " "
    full_name += info[:middle_name]
  end
  full_name += ' '
  full_name += info[:last_name]
  full_name
end

Ruby style dictates we use 2 spaces per level of indentation. The style guide is here, if you're interested.

Moreover this is needlessly wordy. You can replace the entire thing with this:

def full_name
  [:first_name, :middle_name, :last_name].map do |part|
    info[part].to_s
  end.join(" ")
end

If you don't understand what this is doing just comment and I'd be happy to explain. We're taking advantage of the fact that Ruby handles collections of things incredibly gracefully.

count = 0
transac_counter = count += 1

count = 0
transac_counter = count += 1

def full_name
    full_name =info[:first_name]
if !@middle_name.nil?
    full_name += " "
    full_name += info[:middle_name]
end
    full_name += ' '
    full_name += info[:last_name]
    full_name
end

Your indentation is messed up here, this should be

def full_name
  full_name =info[:first_name]
  if !@middle_name.nil?
    full_name += " "
    full_name += info[:middle_name]
  end
  full_name += ' '
  full_name += info[:last_name]
  full_name
end

Ruby style dictates we use 2 spaces per level of indentation. The style guide is here, if you're interested.

Moreover this is needlessly wordy. You can replace the entire thing with this:

def full_name
  [:first_name, :middle_name, :last_name].map do |part|
    info[part].to_s
  end.join(" ")
end

If you don't understand what this is doing just comment and I'd be happy to explain. We're taking advantage of the fact that Ruby handles collections of things incredibly gracefully.

count = 0
transac_counter = count += 1

Source Link

answered Jun 3, 2015 at 18:28

Devon Parsons

1.1k
6
16

Flambino gave you some design points to ponder, so I'll critique your style.

...
if (1..6).include?(ans) 
else
...

You should never have an empty if or else block. Replace this with unless(1..6).include?(ans)

if !@middle_name.nil?
...
if !info.nil?

This is needlessly complicated, just use

if @middle_name
...
if info

count = 0
transac_counter = count += 1

You don't even use these two variables, delete them.

puts"--------------Menu-----------------"
puts"1)Open an Account"
puts"2)View Balance "
puts"3)Make Transaction"
puts"4)View Transaction"
puts"5)Change Pin" 
...

First of all, put a space after puts, but more importantly, either use a heredoc for large multiline text or read the text from a file menu.txt.

total=all.sum

Again, use spaces. This should be

total = all.sum

            if (11410..12000).cover?(zip_code)
                address[:county] = "New York"
            else 
                puts "Unfortunately we don't serve your community yet"
                exit
            end

...


               if amount > origin
                puts "Low balance "
                exit

Your code does a whole lot of exiting. Why not replace that exit with a call to the main menu?

You can't do that at the moment because your Run module doesn't have any methods in it. Aside from the include Transaction, throw all of that into a start or run method or something so you can reuse it.

@@account_list = []
@@count = 0

A final thing to consider when you get more familiar with Ruby - do not use Class variables, use Class instance variables. Class variables have wonky behaviour with inheritence.