Machine gun with multiple firing modes

Question

I have a class 'M249' it is initiated with 'ammo' The class has one responsibility(I hope) to expend ammo. Does there exist a way to increment without accessor or writer?

I am shaky on when I should be using mixins, or inheritance. I think module is best here?

module MG_tools
 Rof = {:sustained => rand(3..9), :rapid => rand(5..9), :cyclic => 
rand(1..200), :cease => 0}
end
class M249 
include MG_tools
 attr_accessor :ammo
 def initialize( ammo )
 @ammo = ammo
 end
 def fire(command)
 self.ammo -= Rof[command]
 raise ArgumentError.new("Out of Ammo!") if self.ammo < 1 
 end
end
command = :sustained
gunner = M249.new(200)
loop do 
case gunner::ammo
 when (200..1000)
 command = :cyclic
 when (100..199)
 command = :rapid
 when (6..99)
 command = :sustained
 when (1..5)
 command = :cease
end
 puts "#{command.upcase} FIRE"
 break if command == :cease
 gunner.fire(command)
 puts "*MACHINE GUN NOISES*"
 puts "SAW: #{gunner::ammo} rounds left!"
end

Answer 1

This is an example of a more OOP way of solving your problem.

Single Responsibility Principle

I don't think you can quite answer this question without also asking: where the changes are going to come from – and who is going to ask for those changes. In any case I've taken liberties and refactored and refactored to eliminate as many code smells as I code. So this is one way to solve your problem. Notice that there is a class in your gun called "SmartAmmo". This object knows how to react when ammo goes from 200 to a lesser amount. You will note that each object knows how to act. We are not saying:

if ammo == 10 then do_x_y_z # that's a no-no

we want to be able to select the appropriate object for the job and we want it to be able to work. the only time we see a conditional is in the "factory method" where they are all created.

Please study this code very carefully, it certainly took some time to get it done for you, and if you feel it is worthy, please promote it so I get some reputation points :)

There is one thing I didn't implement - that is the randomness involved when going from sustainted to rapid to cyclic modes. I've left the implementation of that element of randomness as an exercise for the reader.

I really hope you enjoy the code :)

https://gist.github.com/BKSpurgeon/8774c855926818e2ecd141198273ff47

(It's in a gist because StackOverflow formatting is horrible).

class Gun
 attr_reader :smart_ammo
 def initialize(ammo) 
 @smart_ammo = SmartAmmo.at(ammo)
 end
 # how it sounds from start to finish
 def sound_of_the_whole_nine_yards
 sounds_of_gunfire = ""
 loop do
 # concatenate all the shots
 sounds_of_gunfire = sounds_of_gunfire + @smart_ammo.fire
 @smart_ammo = @smart_ammo.next_ammo
 break if @smart_ammo.ammo <= 0
 end
 puts sounds_of_gunfire
 end 
end
class SmartAmmo
 attr_reader :ammo
 def initialize(ammo)
 @ammo = ammo
 end
 def self.at(ammo)
 case ammo
 when 200..1000 
 CyclicAmmo.new(ammo)
 when 100..199 
 RapidAmmo.new(ammo)
 when 6..99 
 SustainedAmmo.new(ammo)
 when (1..5) 
 CeaseAmmo.new(ammo)
 else
 SmartAmmo.new(ammo)
 end
 end
 def next_ammo
 SmartAmmo.at(quantity_remaining_inventory)
 end
 def fire
 "#{speed}" + "#{output_remaining_inventory_after_consumption}" 
 end
 def speed
 end
 def quantity_remaining_inventory
 end
 def output_remaining_inventory_after_consumption 
 "SAW: #{quantity_remaining_inventory} rounds left!\n" 
 end
end
class CyclicAmmo < SmartAmmo
 def speed
 "CYCLIC FIRE\n" + "*MACHINE GUN NOISES*\n" 
 end
 def quantity_remaining_inventory
 @ammo - 5
 end
end
class RapidAmmo < SmartAmmo
 def speed
 "RAPID FIRE\n" + "*MACHINE GUN NOISES*\n" 
 end
 def quantity_remaining_inventory
 @ammo - 8
 end
end
class SustainedAmmo < SmartAmmo
 def speed
 "SUSTAINED FIRE\n" + "*MACHINE GUN NOISES*\n" 
 end
 def quantity_remaining_inventory
 @ammo - 4
 end
end
class CeaseAmmo < SmartAmmo
 def speed
 "CEASE FIRE\n"
 end
 def quantity_remaining_inventory
 -1
 end
 def output_remaining_inventory_after_consumption 
 end
end
Gun.new(200).sound_of_the_whole_nine_yards

Answer 2

Without more code / context it is hard to give much of an answer, but a few comments:

• You can make ammo a read only attribute by using attr_reader :ammo then inside the class or module you can access it using @ammo just like you do in your initializer.

• Don't over-complicate your code. Given what you have now there is no reason to do any of what you want. Wait till your code gets complicated enough that you need to break it up then you won't just be guessing what the best option is.

• Consider using delegation rather than inheritance or modules. So your gun could have an ammo_clip attribute referencing an AmmoClip instance.

• This code:

  Rof = {:sustained => rand(3..9), :rapid => rand(5..9), 
   :cyclic => rand(1..200), :cease => 0}
  

probably doesn't do what you expect. Those random numbers will be calculated once at program startup, not each time a gun is fired.

• user135185: I thought it bad practice to access instance variables that way def fire(command) @ammo -= Rof[command] raise ArgumentError.new("Out of Ammo!") if self.ammo < 1 Referencing an instance of an object that holds ammunition seems very ideal rather than a module or inheritance. I am unaware of how to fix the Rof hash I had it in my original post that I am unsure of how to make those numbers unique to each fire. I know this is a seperate issue/question My loop / case statement were meant to be a 'test' not functional code I am very new to tdd/ood