0

Here is a ruby array:

array = ['x', 3, 0, 4, 4, 7]

I want to map through array, take every integer in the array (except for 0 ) minus 2, and return a new array.

When there are letters, no change will be made to the letters.

Example output:

['x', 1, 0, 2, 2, 5]

This is what I have, but I got the error message saying "undefined method integer? "can someone tell me what is the problem?

    def minusNumber(array)
     array.map do |e|
      if e.integer? && e !== 0
      e - 2
      end
     end
    end
Improve this question
2
  • 2
    Use is_a? to check the object's class, as integer? isn't a method for strings, that's why the NoMethodError. You'd need an else statement, perhaps. And to replace !==. Commented Jul 4, 2018 at 21:37
  • You have made no specification for non-letter Strings what should "0" or "2" return? They are not Integers but they are not "letters" either. (for the sake of the question I will assume that punctuation and other non-numeric/non-letter characters should be considered "letters") Commented Jul 5, 2018 at 13:50

6 Answers 6

Reset to default
2

The other answers here will work fine with your current input. Something like:

def minusNumber(array)
  array.map do |e|
    if e.is_a?(Integer) && e != 0
      e - 2
    else
      e
    end
  end
end

But here is a more flexible solution. This might be a bit too advanced for you where you are now, but all learning is good learning :-)

Ruby is a language that allows polymorphism in its variables. You can see, using the example input for your method, that the variable e may contain a String object or an Integer object. But it actually may contain any type of object, and Ruby would not care one bit, unless it encounters an error in using the variable.

So. In your example, you need to keep Integers in the output. But what if in the future you need to pass in an array containing some Score objects, and you'd need those in your output too? This is a brand new class that you haven't even written yet, but you know you will later on down the line. There's a way you can re-write your method that will anticipate this future class, and all other Integer-type classes you may someday write.

Instead of using #is_a? to check the type of the object, use #respond_to? to check what methods it implements.

Any class that can be used as an integer should implement the #to_int method. Integer certainly does, and your future Score class will, but String does not. And neither does any other class that can't be considered like an integer. So this will work for all types of values, correctly separating those that respond to #to_int and those that don't.

def minusNumber(array)
  array.map do |e|
    if e.respond_to?(:to_int) && e != 0
      e - 2
    else
      e
    end
  end
end

Again, this stuff might be a bit advanced, but it's good to get in the habit early of thinking of variables in terms of its methods as opposed to its type. This way of thinking will really help you out later on.

Improve this answer
Sign up to request clarification or add additional context in comments.

10 Comments

Asking for :to_int is not enough. e would also need to respont to :-. For this task, asking to where e responds to, is IMO not appropriate anyway, because the requirement says that the transformation should apply if e is an Integer, not if it can be turned into one. Imagine someone one would Monkey-patch String to also have a to_int method.... What would make sense, is to test for e.class <= Integer instead of e.is_a?(Integer).
If it responds to :to_int it will respond to :-. Implementing #to_int is a contract that the writer of the class makes to its users that says, "this can be considered an Integer in ALL cases where an Integer can be used". Inheritance is the best, but not the only, way of abiding by this contract.
@user1934428 why would you recommend <= over is_a?. is_a? has a simpler implementation and provides the desired result in this context. e.g. the OP does not need to know if Integer is less than e.class (false) or not in the ancestry chain at all (nil) and given the fact that e is an instance already e.is_a? makes more logical sense since the desired result from is_a? and <= is an identical call to class_search_ancestor without all the other contextual checks that <= implements
@engineersmnky: :to_i is absolutely not the message to check a response to; String implements :to_i, and the use-case specifically states to return strings as-is. See this answer for more about the differences between :to_i and :to_int: stackoverflow.com/a/11182123/139299
@engineersmnky: We're arguing over a spec that hasn't had its edge-cases sanded off. I think we're both right, depending on what the client wants :-). The purpose behind my answer was to help a newbie start to think of objects not as having a fixed type based on their class, but a flexible type based on their public interface. It's something I wish I'd learnt sooner.
|
2

Here's another take on this:

array = ['x', 3, 0, 4, 4, 7]

transformed = array.map do |e|
  case e
  when 0, String
    e
  when Integer
    e - 2
  else
    fail 'unexpected input'
  end
end

transformed # => ["x", 1, 0, 2, 2, 5]

It's a pity that you have to keep the elements from which you didn't subtract 2. I really wanted to do something like this

array.grep(Integer).reject(&:zero?).map{|i| i - 2 } # => [1, 2, 2, 5]

Couldn't find a way yet (which preserves the unprocessed items).

Improve this answer

7 Comments

A case statement seems a bit complex for such a simple operation IMO. Not to say this is not a valid solution and works, just thinking a switch is an expensive operation when only really need to check for numbers, and everything else will be ignored.
@ForeverZer0 It is more expensive how? Performance-wise, it should be identical to an if. And brain-power-wise it should be even cheaper. Reads like engish, no need to decipher any is_a? and == 0. ¯\_(ツ)_/¯
Switch statements inherently have more overhead than if..end in pretty much every language, Ruby being no exception. Not to mention equality comparison works differently, <=> is also more expensive than basic equality, so it is in multiple ways.
@ForeverZer0: no <=> here, though
@ForeverZer0: nope, they don't. They use threequals, not the spaceship. Benchmarked mine against yours and yes, yours is 1.5x faster. Still I think this one reads better.
|
0

You can do this without checking for type.

use of kind_of? is a code smell that says your code is procedural, not object oriented… https://www.sandimetz.com/blog/2009/06/12/ruby-case-statements-and-kind-of

def someMethod(arr)
  arr.map do |item|
    next item unless item.to_i.nonzero?
    item - 2
  end
end

someMethod(["x", 3, 0, 4, 4, 7])

> ["x", 1, 0, 2, 2, 5]
Improve this answer

2 Comments

Honestly I would say this is the best answer excepting that "2".to_i.nonzero? #=> true but the fallout is a NoMethodError because String#- is not a method and even if it was it is unlikely it would accept an Integer as its argument
item.inspect.to_i.nonzero? works in that case. Although, IMHO, I don't think it's ideal. I feel uncomfortable about coercing item to a string and back to an integer and I don't think it's clear why or what it's doing anyway.
0

Short and sweet. 

array.map { |v| v.is_a?(Integer) && !v.zero? ? v - 2 : v }

This eliminates all the needless case statements, multiple iteration, and transformations. Simply takes each element, if it is an integer and greater than 0, then subtracts 2, otherwise does nothing.

Improve this answer

2 Comments

every integer in the array (except for 0), what happens with the number lower than 0?
&& v.nonzero?
-1

As mentioned in the comment above you should use is_a? method to check the datatype of an element. The code below should work as expected: 

 def minusNumber(array)
   array.map do |e|
     if e.is_a?(String) || e == 0
       e
     else
       e - 2
     end
   end
 end

Instead of the if statement you could also do:

(e.is_a?(String) || e == 0 ) ? e : e-2

Please note that the original object is unchanged unless you use map!

Improve this answer

Comments

-1

I think this is legible and elegant:

array.map { |v| v.is_a?(Integer) && v == 0 ? v : v -2 }
Improve this answer

2 Comments

Lol, almost cut-paste of the solution I had already posted stackoverflow.com/questions/51180923/…
Not that it generally matters anymore and especially in a ternary situation but a good rule of thumb is that a conditional should check the "most likely" scenarios in order so as to reach the result in the fastest manner. This is especially true with if..elsif conditionals. Thus given the provided input we can divide the results into 2 groups ['x',0] and [1,3,4,4,7] as you can see the second group is larger and thus "more likely" to occur so we should check for this and let group one represent false in this case. However your example checks for group 1.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.