Change GetAttributes to GetAllAttributes

As a developer it is too difficult to differentiate between GetAttribute and GetAttributes as they are too similar with only one character difference it has a lot of potential for typos and cause a lot of bugs.

GetAttribute returns a variant of values while GetAttributes returns a table, yet they have very similar names

So I would suggest changing GetAttributes to GetAllAttributes it is a lot more distinctive.
(could either deprecate GetAttributes or just add GetAllAttributes)

GetAttributes should be changed to GetAllAttributes
  • I agree
  • I disagree

0 voters

If Roblox were to add this then it would give us a lot less to worry about when using Attributes

3 Likes

That naming wouldn’t really be consistent with any other engine API that returns a collection of all things under a certain object. There’s GetChildren, GetTags, GetDescendants, etc.

You should be able to find out that you made a typo from script editor type warnings if you use GetAttributes with a string argument or GetAttribute without any.

18 Likes

I agree but we don’t have GetChild, GetTag nor GetDescendant so they aren’t really an issue.

The problem that I have with GetAttribute and GetAttributes is that they are very similar to each other.

I agree that a proper Script Editor would provide us with a warning and a good developer should always unit test their code.

With that said I was writing code in Roblox Studio instead of VSCode because Rojo doesn’t support Attributes yet, and Roblox Studio didn’t give me that warning, also new developers don’t unit test their code :upside_down_face:

So most of my concern is for new developers rather than advance ones, because we have tools like Selene while they don’t

I think that is an understatement

I do not believe attributes have been around long enough to worry about this, and if a beginning developer makes this mistake they can fix it themselves by realizing that the function they used does the wrong behavior, and if they really can’t find it #help-and-feedback:scripting-support is at their service. I don’t think this is necessary, even as just an alias.

2 Likes

How about we wait until people have genuine experiences making this mistake and not realizing immediately before we jump to conclusions about whether or not this naming will cause this issue. It’s not worth breaking API consistency because of an unconfirmed fear.

2 Likes

I think it makes more sense to just deal with debugging this. Calling :GetAttribute() without arguments should error 100% of the time, whereas :GetAttributes() would be expected not to error at all. Even if you have attributes with a table that could potentially be looped over, you’d have to be passing the attribute name. I don’t think this makes sense.

It’s the same difference between accidentally typing :GetTag() vs :GetTags(), the prior always errors, the latter never does.

If the argument is readability, I think the same thing applies. If you are passing an argument it should be assumed that it is :GetAttribute() singular not :GetAttributes() and vice versa.

1 Like