Skip to content

Instantly share code, notes, and snippets.

@nicusg
Last active December 21, 2015 10:18
Show Gist options
  • Save nicusg/6290804 to your computer and use it in GitHub Desktop.
Save nicusg/6290804 to your computer and use it in GitHub Desktop.
Rewritten some methods
class Player
@stairs = false
@wall = false
@warrior
def play_turn(warrior)
@warrior = warrior
if damaged_from_behind
warrior.shoot!(:backward)
else
should_look
end
@health = warrior.health
end
def can_fight
@warrior.health > 15
end
def taking_damage?
@warrior.health < @health
end
def injured
if taking_damage?
return @warrior.walk!(can_fight? ? :forward : :backward)
else
@warrior.rest!
end
end
def explore
if @warrior.feel.stairs?
@stairs = true
return @wall ? @warrior.walk! : @warrior.pivot!
end
if @warrior.feel.wall?
@wall = true
return @warrior.pivot!
end
@warrior.walk!
end
def first_non_empty(spaces)
spaces.each do |space|
return "enemy" if space.enemy?
return "captive" if space.captive?
end
"blank"
end
def damaged_from_behind
@warrior.health < 20 && taking_damage? && first_non_empty(@warrior.look(:backward)) == "enemy"
end
def check_health
@warrior.health < 20 ? injured : explore
end
def should_move
return @warrior.attack! if @warrior.feel.enemy?
return @warrior.rescue! if @warrior.feel.captive?
check_health
end
def should_look
if first_non_empty(@warrior.look) == "enemy"
@warrior.shoot!
else
should_move
end
end
end
@nicusg
Copy link
Author

nicusg commented Aug 21, 2013

@timsegraves @ben-grid @ninetwentyfour can you please take a look?

@timsegraves
Copy link

Play turn looks much better now!

@timsegraves
Copy link

Totally optional but you typically see a lot of ternary operators in ruby if they conditionals are simple. I might write check_health as:

def check_health
  @warrior.health < 20 ? injured : explore
end

Some people don't like those but I usually do if the check is simple.

@timsegraves
Copy link

I might also write should_move as:

def should_move
  return @warrior.attack! if @warrior.feel.enemy?
  return @warrior.rescue! if @warrior.feel.captive?

  check_health
end

Once again that's just a stylistic different (i.e. no performance difference) but I think it makes it read more like a sentence than the classic if statement.

@timsegraves
Copy link

The nested if statements in injured and explore still bug me a bit. Not a huge deal but I might approach explore like:

def explore
  @stairs = @warrior.feel.stairs?
  @wall = @warrior.feel.wall?

  @wall ? @warrior.pivot : @warrior.walk
end

That said, I simplified the logic you had there a bit. I'm not sure why if you feel stairs you then check to see if there is a wall and walk if there is a wall but pivot if there is not. That doesn't make much sense to me. I might be missing something though.

@timsegraves
Copy link

Injured I might write something like:

def injured
  if @warrior.taking_damage
    return @warrior.walk!(can_fight? ? :forward : :backward)
  else
    @warrior.rest!
  end
end

@timsegraves
Copy link

That's pretty much it. Sorry to just show you how I'd do it but sometimes its hard to make suggestions until I sit down and write through the code. Most of them are just syntactic sugar that ruby makes easy. Let me know if you have any questions.

@timsegraves
Copy link

Nice, that looks better.

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