MushroomObserver / mushroom-observer / 1178
93%
main: 95%

LAST BUILD BRANCH: 3312-inat-import-preview
DEFAULT BRANCH: main
Ran
Jobs 1
Files 524
Run time 27s
Badge
Embed ▾
README BADGES
x

If you need to use a raster PNG badge, change the '.svg' to '.png' in the link

Markdown

Textile

RDoc

HTML

Rst
1178

push

travis-ci

web-flow 
Delete obsolete TODO in AbstractModel

This was fixed 5 years ago.  See following email chain:
>Nathan Wilson to me, Jason
Jun 24
Sounds reasonable to me, but I sure don't remember writing that code!
…
On Thu, Jun 22, 2017 at 9:25 PM, Joseph Cohen <jdcohenesq@gmail.com> wrote:
My guess:  Nathan added the TODO while he was working on it, but just forgot to remove it once everything passed and he was done.
...
On Thu, Jun 22, 2017 at 6:23 PM Jason Hollinger <pellaea@gmail.com> wrote:
Ah!  Now all makes sense.  I think the TODO was actually done.  I think the present state is ideal -- there is a default which works for most controllers, but instead of switch statements (a good sign that we aren't using subclasses correctly) it does the overrides explicitly in the aberrant subclasses.  Perfect.
...
Only question... why is the "TODO" still there?? :)
...
On Thu, Jun 22, 2017 at 9:16 PM, Joseph Cohen <jdcohenesq@gmail.com> wrote:
Jason:
Thanks for the response.
...
I think the comment about the "default is an error" refers to the prior definition: 
  def self.show_controller
-    case name
-      when 'Observation', 'Naming', 'Vote', 'User', 'RssLog'		
 -        return 'observer'		
 -      when 'Comment', 'Image', 'Location', 'Name', 'Project', 'SpeciesList'		
 -        return name.underscore		
 -      when /Description$/		
 -        return $`.underscore		
 -      else		
 -        raise(ArgumentError, "Invalid object type, \"#{name.underscore}\".")		
 -    end		
    end
>
And I now suspect that TODO refers to: ditching the case statement, putting a general definition in abstract_controller, and overriding it in any classes where necessary. In any event, I'm finding it handy to have the definition abstracted.  Do you think I should get rid of the abstract definition and define it in explicitly in all the models where it's not now explicitly defined?  Seems anti-object oriented.
...
>
On Thu, Jun 22, 2017 at 5:09 PM Jason Hollinger <pellaea@gmail.com> wrote:
I assume that it is considered better to have each subclass define this explicitly, than to have the base class "calculate" it.  I have no idea what the comment about the "default is an error" means.  Makes no sense in the context here, in any case...
>...
On Thu, Jun 22, 2017 at 7:13 PM, Joseph Cohen <jdcohenesq@gmail.com> wrote:
Nathan, Jason:
>
Can either of you elaborate on the following TODO about AbstractModel#show_controller?  (The comment was added about 5 years ago in Commit #955077d0.)
>
```ruby
  # Return the name of the controller (as a simple lowercase string)
  # that handles the "show_<object>" action for this object.
  #
  #   Name.show_controller => "name"
  #   name.show_controller => "name"
  #
  # TODO: Make this a model method!  Also it"s not clear why the default
  # is an error rather than name.underscore.
  def self.show_controller
    name.underscore
  end

  def show_controller
    self.class.show_controller
  end
```

34683 of 37164 relevant lines covered (93.32%)

190.74 hits per line

Jobs
ID Job ID Ran Files Coverage
1 1178.1 (2.2.3) 0
93.32
 Travis Job 1178.1
Source Files on build 1178
Coverage ∆ File Lines Relevant Covered Missed Hits/Line