2
\$\begingroup\$

This is probably silly, but I feel like there should be a much more elegant way to do this simple thing. I have two known strings, whichever one is passed in in the parameter I want to return the opposite. Current code:

def matching_sub(subscription_type)
  if subscription_type == "maths"
    "reading"
  else
    "maths"
  end
end

I came up with:

def matching_sub(subscription_type)
  (%w{reading maths} - [subscription_type]).first
end

and my colleague did:

def matching_sub(subscription_type)
  {"maths" => "reading",  "reading" => "maths"}[subscription_type]
end

Can anyone top that?

\$\endgroup\$
  • 3
    \$\begingroup\$ Which value you want to return by default? Honestly, I'd write subscription_type == "maths" ? "reading" : "maths" and be done with it... \$\endgroup\$ – tokland Mar 3 '15 at 7:10
3
\$\begingroup\$

Your first two solutions both suffer when given unexpected input. The first is biased towards returning "maths" as a default; the second tends to return "reading".

In contrast, your colleague's function returns nil, which is probably more appropriate. I quite like that straightforward lookup, with a minor drawback being that each string is hard-coded twice.

Here's one more suggestion. This one crashes with TypeError: nil can't be coerced into Fixnum when given invalid input. You should decide whether that exception is beneficial or not for your purpose.

SUBJECTS = %w(reading maths)

def matching_sub(subscription_type)
  SUBJECTS[1 - SUBJECTS.index(subscription_type)]
end
| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

200_success' answer is pretty great; this is just a further alternative

SUBJECTS = %w(reading maths).freeze

def matching_sub(subscription_type)
  (SUBJECTS - [subscription_type]).first
end

This handles nils, but is instead biased in case of unexpected input.

But it's too complicated if there really are only two subjects. However, this basic approach will work with more than two subjects, should any be added later or whatnot (I could imagine there's more than maths and reading in this world).

For instance:

SUBJECTS = %w(reading maths music sports).freeze

def missing_subjects(*subscriptions)
  SUBJECTS - subscriptions.flatten
end

Here, you can call missing_subjects("music", "maths") and it would give you ["reading", "sports"] back.


Small note: In your current code, I'm not a fan of calling the method matching_sub. For one, there's the abbreviation, but more importantly, the method actually returns the opposite of a "matching subscription". Hence why I called the method something else above.

| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

It seems like you have (at least) two distinct types of subscriptions. Instead of representing the subscription type as a String, you could leverage the type system and duck-typing like so.

class Subscription; end

class ReadingSubscription < Subscription
  def self.matching_subscription
    MathsSubscription
  end
end

class MathsSubscription < Subscription
  def self.matching_subscription
    ReadingSubscription
  end
end

These could also be instance methods and return appropriate instances, of course.

This solution might be a little bit "over engineered", depending on the context. If you have some sort of Subscription class with behaviors attached, this might be a good solution. Otherwise I would go with one of the simpler solutions (and maybe use a Symbol instead of a String).

| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

No bias, one line, handles any input gracefully:

def get_next(key, group)
  group.is_a?(Array) && (group + [group[0] , nil]).inject{|p,c| return c if p == key;c}
end

subjects = ["math", "reading"]
p get_next("math", subjects)    # => "reading"
p get_next("reading", subjects) # => "math"
p get_next("science", subjects) # => nil


colors = ["red", "yellow", "green", "blue"]
p get_next("red", colors)        # => "yellow"
p get_next("yellow", colors)     # => "green"
p get_next("green", colors)      # => "blue"
p get_next("blue", colors)       # => "red"
p get_next("heliotrope", colors) # => nil


indices = [43,46,:glossary]
p get_next(43, indices)        # => 46
p get_next(46, indices)        # => :glossary
p get_next(:glossary, indices) # => 43
p get_next(nil, indices)       # => nil


p get_next("anything", nil) # => false
p get_next(nil, "string")   # => false

Returns nil when the search fails or false when the group param is not an array. If you are confident in your ability to pass in Arrays only, you can clean it up by getting rid of the call to is_a?

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.