3

To begin: This is my first attempt at getting business logic out of the Model/controller space. Here is some initial logic I'm trying to abstract. The path is app/services/Date_calc.rb.

class Date_calc
  require User
  require Report

  def months(range)
    User.first.reports.order("report_month desc").limit(range).pluck(:report_month)
  end
end

In my application I have two models, User and Reports. User has_many Reports. The reports table has a field called report_month.

Calling Date_calc.months(6) in the Rails console returns: TypeError: no implicit conversion of Class into String.

My intended response was an array of dates, e.g. ["01/01/2013", "01/02/2013", "01/03/2013", ... ].

I'm not really sure what I'm doing wrong here.

Jordan Running
  • 102,619
  • 17
  • 182
  • 182
greyoxide
  • 1,197
  • 1
  • 17
  • 45

2 Answers2

3

The problem is that require expects a String (e.g. require "path_to_module") but you're giving it a Class:

require User
require Report

If this is running in Rails, then Rails will autoload both of these classes. You don't need to do require at all.

Edit: After you've removed those lines, Date_calc.months(6) is still going to give you a NoMethodError, because months is an instance method, but you're trying to call it as though it's a class method. You either need to call it on an instance of the Date_calc class, like this:

Date_calc.new.months(6)

Alternatively, you could define it as a class method by doing def self.months(range) instead of def months(range), which would allow you to call Date_calc.months(6). But you might want to think about whether or not Date_calc should actually be a class, or if it should be a module.

It should be a class if you want to have multiple instances of it, like you would want an instance of a User or instance of a Report. "User" and "Report" are both nouns, because the class represents a thing that we might want to have more than one of. Does the sentence "I want to create a Date_calc" make sense? How about the sentence "I want to create two Date_calcs"? If you said yes to both, then a class makes sense.

If, however, you just want something to put some related methods in, but that doesn't represent a thing, then you might want to use a module instead. In Rails, for example, the Rails object is not a class, it's a module. It doesn't make sense to say "I want to create a Rails," much less "I want to create two Rails." Rails is just a module used to group related methods together. If Date_calc fits that description, then it makes more sense as a module:

module Date_calc
  def self.months(range)
    # ...
  end
end

# and then...
Date_calc.months(6)

As you can see, we use def self.months instead of def months, because we want to be able to call the method on the module itself.

Jordan Running
  • 102,619
  • 17
  • 182
  • 182
  • Thats what I tried at first, which yields: NoMethodError: undefined method `months' for Date_calc:Class – greyoxide Dec 09 '14 at 20:04
  • That error is because of the way you're calling `months`, not because Date_calc hasn't been included (it has). I've updated my answer to explain. – Jordan Running Dec 09 '14 at 20:18
  • Thanks Jordan, having only recently accomplished 'Post.all', this one was baffling. Your comments were the answer i needed. – greyoxide Dec 09 '14 at 21:06
3

In addition to the require lines lacking quote marks, you are calling an instance method on a class. You either need to instantiate the Date_calc like this

months = Date_calc.new.months(6)

or make it a class method like this

def self.months(range)...
msergeant
  • 4,771
  • 3
  • 25
  • 26
  • the ".new" did the trick. And Ive seen this [elsewhere](https://blog.engineyard.com/2014/keeping-your-rails-controllers-dry-with-services) but since the subject was adding users it seemed like they were calling a new record, similar to User.new, it didnt occur to me. – greyoxide Dec 09 '14 at 20:11
  • It works, but does it make sense? What does `Date_calc.new` really mean? What are you creating an instance of? – Jordan Running Dec 09 '14 at 20:28