Sketchup::View#camera=() needs a less confusing "sister" method

[DanRathbun]

SketchUp API, all version, both platforms.

---

EDIT: Changed this issue to recommend a "sister" clone method with no = in the name be coded to better handle multiple arguments and always return a new camera reference (allowing call chaining.) (<<See 3rd post below>>)

---

The API docs are a bit confusing (especially to those not familiar with YARD output.)

When a coder wants to specify a transition time, they expect that the API would be smart enough to detect the 2nd argument and honor it.

Instead the result is that the 2nd argument is just ignored (ie, a silent failure.)

The view#camera= method is an overloaded method, which currently can be used two ways:

1. with a single Sketchup::Camera object argument

2. with a single Array argument of: [ Sketchup::Camera, Float ] (The Float being the transition time.)

A. This enforcing of multiple arguments to be passed as an array is ludicrous as the argument list is passed to the C-side as an array anyway! [EDIT: Mainly caused by Ruby not dealing with multiple arguments for setter methods well. So we won't blame the API here.]

B. This is "outside of the norm" for the API. (Most other methods allow a varying number of arguments.)

This method can be fixed to take 1 object, 1 array or, multiple arguments without breaking plugins "in the wild."

[DanRathbun]

Reference a couple of public forum threads showing the confusion this method implementation causes coders:

• SketchUp Forums: Setter #view.camera= Testing with Second Transition Time Argument

• SketchUcation Forums: sdmitch on using a transition with view#camera=

[DanRathbun]

I can see that the MAIN issue here is really Ruby's treatment of methods using "=" sign character. Ruby does not like multiple arguments for "setter" methods.

It seems to me (after more thought) the best solution is to promote the use of ONE camera object argument for view.camera= going forward, ...

... and create a "baby sister" method clone WITHOUT an = character in the name, that takes multiple methods in the normal way, and calls it's "big sister" (but returns a new camera reference as set):

Example:

Sketchup::View.class_eval {
  define_method(:camera_set_to) do |*args|
    case args.size
    when 0
      fail(ArgumentError,'Wrong number of arguments (0 for 1 or 2)',caller)
    when 1
      self.camera=(args.first)
    else
      self.camera=(args) # ignores unused arguments
    end
    self.camera
  end
}

SO, @thomthom this issue should most likely be labeled as "Enhancement" as it's is really more suited as a new "sister" method request.

I'm renaming the issue to: Sketchup::View#camera=() needs a less confusing "sister" method.

---

The Sketchup::View#camera=() method return value follows the Ruby "setter" rule of returning the arguments given to the setter. When 1 camera argument is passed the camera argument is returned. When a 2 argument array is passed, that array is returned.

But these references are most likely of not much value after the setter returns, ... and coders are more likely to want a new camera object for the current camera returned. This would allow for call chaining.

Hence the statement at the end of the method, calling the view's current camera object to return a new camera reference as set by the new method.

Examples:

new_cam = view.camera_set_to( pages[0].camera, 3.0 )

.. or ...

fl = view.camera_set_to( pages[0].camera ).focal_length

~

[DanRathbun]

If you'd like to try it out as a Ruby 2.x+ Refinement ... (SketchUp 2014+) ...

module Refined
  module View

    refine Sketchup::View do

      def camera_set_to(*args)
        case args.size
        when 0
          fail(ArgumentError,'Wrong number of arguments (0 for 1 or 2)',caller)
        when 1
          self.camera=(args.first)
        else
          self.camera=(args) # ignores unused arguments
        end
        self.camera # Always return a ref to the new camera!
      end

    end

  end
end

Usage is like this:

using Refined::View
# with the current page at pages[0] (first page)
view = Sketchup::active_model.active_view
pages = Sketchup::active_model.pages
cam1 = pages[-1].camera # last page's camera
cam2 = view.camera_set_to( cam1, 3.5 )

:geek:

[thomthom]

Logged as SU-38428

[Eneroth3]

"I can see that the MAIN issue here is really Ruby's treatment of methods using "=" sign character. Ruby does not like multiple arguments for "setter" methods."

I would say the main problem is that the designer of the API chose to use a Ruby setter method, not that the Ruby setter method is bad in itself. It doesn't make much sense to set a state of an object to anything other than a single object (a single parameter). I would argue the API misuses the Ruby setter method; you don't really set the camera to be an Array of a camera and a float. An Array isn't a camera.

I agree completely with Dan that a new method would make this much more sensible. I would name the the sister method set_camera to be consistent with Entity#set_attribute though.

[DanRathbun]

I would name the the sister method set_camera to be consistent with Entity#set_attribute though.

Consistency is best!

@thomthom I would not argue agin' this.