Toggling a taxi between busy and available states Is there a better way to do this? That switch_status method seems like it could be made a lot more compact.

class Taxi
  def initialize
    @status = :available
  end  

  def switch_status
    if @status == :available
      @status = :busy
    else
      @status = :available
    end
  end
end

t = Taxi.new
t.switch_status
I voted to reopen, though I still don't consider this a good CodeReview question. Perhaps my answer here will shed some light on why I think that.

You say you want to shorten the switch_status method. Well, Ruby has ternaries, same as many other languages so:

def switch_status
  @status = (@status == :available ? :busy : :available)
end


And done. Shorter.

But the reason this question feels incomplete is because that, in and of itself, is completely equivalent to the original code. So it's not really a review, just a demonstration of an alternative if..else syntax found in many languages.

A review would require knowing more about why this method exists. What its purpose is, when you need it, why it's using symbols, and so on.

To start, just having the switch_state method is odd. You presumably have to know what state the taxi (as the class name suggests it is) is already in, in order to see if switching it makes sense. But your code contains no mechanism for determining the current state. In fact, the only way to get a Taxi instance to return something is to change its state. Calling switch_state will return the new state, and you'll have to work backwards from there or switch it back again.

E.g. suppose you have an instance, and want to know its state. Currently, you'd have to do this:

some_taxi.switch_state # flip it once
current_state = some_taxi.switch_state # flip it back again


That makes very little sense.

Also: You're switching between exactly two states - which is what booleans do already. So why aren't you using booleans? They're limited to two states, so you're certain it's either one or the other (whereas a symbol can be anything). So you could just as well do this:

def switch_status
  @busy = !@busy
end


Really, that should do the same trick, since this taxi has only two states.

And also, given the lack of context, I'd say you're entire class could, in effect, be replaced with just a boolean, since the class's only purpose right now is to store a boolean state. So cut out the middleman. A raw boolean value would even tell you its current state, which is more than your class does now.

But presumably you want the encapsulation and the symbols for something. Or do you want symbols? A more Ruby'esque way to inquire about a known set of states would be with ?-methods:

def busy?
  @busy
end

def available?
  !@busy
end


Or you can just have one of 'em and negate their value, e.g. !taxi.busy?. Either way, it's infinitely better than flipping the state twice just to find out what it currently is.

If you still want symbols, write an accessor:

def status
  @busy ? :busy : :available
end


Then again, if this is a taxi, does it really have only those two states? Maybe one is :out_of_service for instance?

Again, no idea. The question doesn't say.

Or it could be that available/busy can be - and likely should be - inferred from something else. There's no reason to manually track multiple separate state values if they're dependent on each other. In fact, it's likely harmful to try. So perhaps your code could do this instead:

def status
  @passengers.any? ? :busy : :available
end


Now status is derived from something (presumably) more meaningful.

But does this make sense? No idea. The question doesn't say.

As mentioned, just having switch_state is odd because presumably there's more to it than that: The state is connected to something. Otherwise, as I also mentioned, you could replace the whole class with a simple boolean value. Only being able to manipulate the state by flipping it seems weird. Like a light switch you can only even flip to the opposite of whatever it is - and you don't know whether that's on or off.