Get_attributes has stopped working

I think I’ll just write a method completely internal to my module and replace the ent.get_attributes(“dict_name”) calls with my get_attributes(ent, dict_name). Seems the most straightforward thing to do. I know it means going through my code with a fine tooth comb to replace all the original calls but notepad++ search will find them.

Here is the code
`require ‘sketchup’
def get_attributes(entity, attribute_dictionary_name)
return nil unless entity.is_a? Sketchup::Entity
return nil unless attribute_dictionary_name.is_a? String
dictionaries = entity.attribute_dictionaries
return nil unless dictionaries
dictionary = dictionaries[attribute_dictionary_name]
return nil unless dictionary
attribute_hash = {}
dictionary.each{|key, value|
attribute_hash[key] = value
}
return attribute_hash
end #of get_attributes

This tests it:-
model=Sketchup.active_model
definitionList = model.definitions
component_definition = definitionList.add “fms_tempy”
component_definition.set_attribute(“fms_Temp”, “fms_Temp_detail_level”, 13)
component_definition.set_attribute(“fms_Temp”, “fms_Temp_width”, 5.4)
attribs_hash = get_attributes(component_definition, “fms_Temp”)# return the attributes as a hash
puts"attribs_hash is #{attribs_hash}"`

Thank you again everyone for all the super helpful comments!

(1) You need to delimit multiline code in the forum with triple backtick lines, so indents are not stripped (and a coding language name on the first delimiter line to color lex the code.)

The single backtick delimiters can only be used within a single line of forum text.

IE:

```ruby

code here

```

So it should lex like so in the forum:

def get_attributes(entity, attribute_dictionary_name)
  return nil unless entity.is_a? Sketchup::Entity
  return nil unless attribute_dictionary_name.is_a? String
  dictionaries = entity.attribute_dictionaries
  return nil unless dictionaries 
  dictionary = dictionaries[attribute_dictionary_name]
  return nil unless dictionary
  attribute_hash = {}
  dictionary.each{|key, value|
    attribute_hash[key] = value
  }
  return attribute_hash	
end #of get_attributes

(2) You never need any external script / ruby file / library, etc. unless there is a dependency for the functionality of the file.

So the

require 'sketchup'

call is frivolous (but not harmful.)

But if at the top of every single file, (for every extension,) it cumulatively can slow SketchUp’s extension loading as the $LOADED_FEATURES array needs to be checked for each call, and comparing strings in Ruby is slow.

So, if you must, I’d suggest putting it only at the top of the extension registrar script (the one in the “Plugins” folder that registers you extension object. This is the file that will cause all the rest of your extension’s file to be loaded, so there is no more need to subsequently call for the requirement of the “sketchup.rb” file.)

1 Like

Julia, I’m not sure if you are commenting upon my example, or the original that @Camlaman was referring to, which BTW, I searched for but could not find. (I seem to remember this old method and previous discussions about it. But without seeing it, I cannot say what it returns upon type mismatch or failure.)

With regard to my philosophy, I follow Ruby convention, not incorrect SketchUp API conventions !


My actual implementation, would raise TypeError exceptions instead of how Camlaman’s just returns nil (in his first 2 method lines.)


raise(TypeError,"Sketchup::Entity expected for argument(1)") unless entity.is_a?(Sketchup::Entity)
raise(TypeError,"String expected for argument(2)") attribute_dictionary_name.is_a?(String)

This helps when debugging or some strange situation occurs and the values passed to the method are unexpected types. The exceptions and their messages tell you immediately that the problem is happening before the method call, and not internal to the method itself.

Returning nil in these situations just makes your job as a programmer testing and maintaining the code harder.


Secondly, my personal preference, is to mimic the core Ruby .to_hash and .to_a methods, which are expected to return a collection object (Hash or Array respectively.) If the operation does not produce valid members for the collection, then the resultant returned collection is expected to be and should be empty.
Ie, it should work like Enumerable.find_all().

The coder would expect to call result.empty? upon the return reference.

Yes, I know there are some API calls that return nil in situations when they should have returned an empty collection, and one of the API team members agreed it was unconventional, but also too late to change it now. (It’d break code “in the wild”.)

Example from the API docs:

Entity.attributedictionaries()

Returns:
attributedictionaries - the AttributeDictionaries object associated with the entity, or nil if there are no attribute_dictionary objects associated with the model. Care must be taken if nil is returned, for example: invoking attribute_dictionaries.length will throw a NoMethodError exception, not return 0.

This is what I mean about returning nil unconventionally. It sets up coders for failure in successive statements.

An empty Ruby collection could have been returned from this method instead, so that subsequent getter methods for specific member dictionaries could return nil or methods like length could return 0. (Even if under the hood, no empty collection object were added to the model database. Ie, just like empty groups being cleaned up when the reference goes out of scope.)

There could have been .any_dictionaries?() and .has_dictionary?() boolean query methods to make entities more Rubyish. (I may have even requested such … but I forget all the requests I’ve made over the years. It is normal to have boolean query methods that do not actually make Ruby create object references.)

One of the most common “gotchas” that occurs when too many scenarios return nil on failure, is the ol’ exception:

Error: #<NoMethodError: undefined method `[]' for nil:NilClass>

… or calling some other Hash method upon the result.

When the result is an empty hash, the [] method call returns nil by default when the key used is not in the hash.
I’d rather test for nil at that time against individual member accesses.

But to each his own.

I know Julia, you are going to say, “But it is not a .to_hash() method, it’s a .get_attributes() method and should mimic the APIishness.”

Perhaps, but I would not actually use the name “get_attributes()”, I’d use something more like “get_hash()”.

It is not coming from Dynamic Components:

ent = Sketchup.active_model.selection[0]
meth = ent.method(:get_attributes)
meth.source_location

returns:

".../sketchup/plugins/su_advancedcameratools/advancedcameratools_main.rbs",
line 145

And I see that:

ent.get_attributes("UnusedDictionaryName") 
# returns {}

Thank you for this advice. I added the call to require sketchup.rb because I was just thinking in case the method was made a module method and in a file on it’s own or with other module methods and this file was the first called.

I also agree it would be nicer for my get_attributes method to return something more useful than nil and to be called get_attributes_as_hash so that it was called what it did.

By the way, how do I get ‘triple backtick’ format to happen?

that explains why I get so many random ‘no method errors’ when DC calls ‘get_attributes’…

ACT is almost always turned off…

it must happen on Make as well, unless they needlessly load ACT for the one method…

john

I guess this is how multi line code should be formatted. Hope it comes out right the other end.

I have renamed the method and added your suggested error messages, plus a bit.

Method code

	def get_attributes_as_hash(entity, attribute_dictionary_name)
		raise(TypeError,"Sketchup::Entity expected for argument(1)") unless entity.is_a?(Sketchup::Entity)
		raise(TypeError,"String expected for argument(2)") unless attribute_dictionary_name.is_a?(String)
		dictionaries = entity.attribute_dictionaries
		raise(TypeError,"Sketchup entity failed to return Sketchup dictionaries.") unless dictionaries
		dictionary = dictionaries[attribute_dictionary_name]
		raise(TypeError,"Sketchup dictionaries failed to return a Sketchup dictionary.")  unless dictionary
		attribute_hash = {}
		dictionary.each{|key, value|
				attribute_hash[key] = value
		}
		return attribute_hash	
	end #of get_attributes_as_hash

Test code starts here.

	model=Sketchup.active_model
	definitionList = model.definitions
	component_definition = definitionList.add "fms_tempy"
	component_definition.set_attribute("fms_Temp", "fms_Temp_detail_level", 13)
	component_definition.set_attribute("fms_Temp", "fms_Temp_width", 5.4)
	attribs_hash = get_attributes_as_hash(component_definition, "fms_Temp")# return the attributes as a hash
	puts"attribs_hash is #{attribs_hash}"
	attribs_hash = get_attributes_as_hash("Jim", 13)# raises Error!

Thank you for the help.

On a separate line before your code:
Three backticks then the word ruby (no spaces anywhere). I’m not sure if ruby is case sensitive, but I know that lower case works.

Then your code

Then on a new line, after the code, again with no white space, three backticks.

Again starting on a new line, the next part of your post.

1 Like

It is not really needed to put the name of the method into the error message,
as Ruby generates a “in #{method_name}:#{line_number}” as the first line of the error backtrace.

1 Like

Of course you are right. I’ll take the name out. I’m so used to using the puts method for debugging where the name stands out if it is in debug string.

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.