4

I have a function that does this:

  def blank_to_negative(value)
    value.is_number? ? value : -1
  end

If the value passed is not a number, it converts the value to -1.

I mainly created this function for a certain model, but it doesn't seem appropriate to define this function in any certain model because the scope of applications of this function could obviously extend beyond any one particular model. I'll almost certainly need this function in other models, and probably in views.

What's the most "Rails Way" way to define this function and then use it everywhere, especially in models?

I tried to define it in ApplicationHelper, but it didn't work:

class UserSkill < ActiveRecord::Base
  include ApplicationHelper
  belongs_to :user
  belongs_to :skill

  def self.splice_levels(current_proficiency_levels, interest_levels)
    Skill.all.reject { |skill| !current_proficiency_levels[skill.id.to_s].is_number? and !interest_levels[skill.id.to_s].is_number? }.collect { |skill| {
      :skill_id => skill.id,
      :current_proficiency_level => blank_to_negative(current_proficiency_levels[skill.id.to_s]),
      :interest_level => blank_to_negative(interest_levels[skill.id.to_s]) }}
  end 
end

That told me

undefined method `blank_to_negative' for #

I've read that you're "never" supposed to do that kind of thing, anyway, so I'm kind of confused.

Community
  • 1
  • 1
Jason Swett
  • 43,526
  • 67
  • 220
  • 351
  • I have a feeling that there is a flaw in your design if you're needing to use this logic in so many places. – user229044 Aug 07 '12 at 19:48
  • Good point. You could be right. The underlying reasoning is that certain things in my app can be rated 0-10 by a user. If the user doesn't enter a rating, the user's rating is not unknown, it's just *known to be blank*, so rather than storing a NULL (which means "unknown"), I want to store a different value, and I chose -1. Suggestions on a better solution welcome (although certainly not relevant to the original question). – Jason Swett Aug 07 '12 at 19:55
  • the nice thing is that nil and blank strings default to 0 when used with ```to_i``` – phoet Aug 07 '12 at 19:58
  • But 0 is a legitimate rating, and a blank or invalid value should not default to the legitimate rating of 0. – Jason Swett Aug 07 '12 at 20:00
  • yeah, you can check against nil/blank values to determine unknown, but just use the values in calculations when calling to_i on it. got it? – phoet Aug 07 '12 at 20:05
  • 1
    I would use null to represent the blank value. That's what it's for. Wether it's blank because it's unknown or blank because it doesn't exist or blank because it's intentionally blank, it's just **blank**. Nil/null is the correct concept for a *blank* value. – user229044 Aug 07 '12 at 21:07
  • I disagree for multiple reasons but I don't wish to debate. – Jason Swett Aug 08 '12 at 14:26

4 Answers4

3

if you want to have such a helper method in every class in your project, than you are free to add this as a method to Object or whatever you see fits:

module MyApp
  module CoreExtensions
    module Object
      def blank_to_negative
        self.is_number? ? self : -1
      end
    end
  end
end

Object.send :include, MyApp::CoreExtensions::Object
phoet
  • 18,688
  • 4
  • 46
  • 74
  • Okay, I think that makes sense. If I'll only ever use it on a string, I'm guessing I would do `module String` instead of `module Object`? And what file would I put this code in? – Jason Swett Aug 07 '12 at 19:51
  • exactly! i usually add an ```extensions``` directory to my app and put stuff like that there. others put them into ```lib/core_ext``` there is no best practice i know of for rails applications. – phoet Aug 07 '12 at 19:57
  • Hmm, I put this in `app/extensions/string.rb` and it didn't work (undefined method), even though I restarted my server. – Jason Swett Aug 07 '12 at 20:06
  • 2
    I would suggest using a Rails initializer to call the Object.send part – mguymon Aug 07 '12 at 20:06
  • @JasonSwett just put it in ```config/initalizers/core_extensions.rb``` and improve when it becomes too crowded – phoet Aug 07 '12 at 20:07
  • Adding this method to object might not be a good OOP practice. If it is added to object then "String" class also will inherit this method. It will be strange for me to see a method name "blank_to_negative" on a string. – Sandeep Aug 07 '12 at 20:13
  • i don't think that this has something to do with OOP, but yes! that's why i added the ```or whatever you see fits``` – phoet Aug 07 '12 at 20:14
  • By the way, I extended the String class in a much simpler way. I did it like this: http://stackoverflow.com/a/2095512/199712 I don't know what the pros/cons are, but the way I did it seems to work fine. – Jason Swett Aug 07 '12 at 20:18
  • Another "by the way": I changed the name of the function to `negative_if_not_numeric`, since, like Sandeep says, `some_string.blank_to_negative` doesn't make very much sense. – Jason Swett Aug 07 '12 at 20:19
  • Plus the original function name was a misnomer, anyway. Should have been `non_numeric_to_negative`. – Jason Swett Aug 07 '12 at 20:19
  • I edited your answer. I hope you don't mind that it was a drastic edition, but now the answer matches what I actually did to resolve my issue. – Jason Swett Aug 07 '12 at 20:24
  • i really dislike your editions. if you found your answer, why not add it to your question. – phoet Aug 07 '12 at 20:31
2

There are a few options:

  • Monkey-patch the method into ActiveRecord and it will be available across all of your models:

    class ActiveRecord::Base
      def blank_to_negative(value)
        value.is_number? ? value : -1
      end
    end
    
  • Add a "concern" module which you then mix into selected models:

    # app/concerns/blank_to_negate.rb
    module BlankToNegate
      def blank_to_negative(value)
        value.is_number? ? value : -1
      end
    end
    
    # app/models/user_skill.rb
    class UserSkill < ActiveRecord::Base
      include BlankToNegate
      # ...
    end
    
user229044
  • 232,980
  • 40
  • 330
  • 338
  • If I were to go with your first idea, would I call it by saying `self.blank_to_negative(value)` or `blank_to_negative(value)`? If the former, I don't feel that that would make complete sense. – Jason Swett Aug 07 '12 at 19:59
1

Ruby Datatypes functionality can be extended. They are not sealed. Since you wan to use it in all places why not extend FIXNUM functionality and add a method blank_to_negative to it.

Sandeep
  • 7,156
  • 12
  • 45
  • 57
0

Here's what I ended up doing. I put this code in config/initializers/string_extensions.rb.

class String
  def is_number?
    true if Float(self) rescue false
  end 

  def negative_if_not_numeric
    self.is_number? ? self : -1
  end 
end

Also, I renamed blank_to_negative to negative_if_not_numeric, since some_string.negative_if_not_numeric makes more sense than some_string.blank_to_negative.

Jason Swett
  • 43,526
  • 67
  • 220
  • 351