My Personal Library [Module]

[colbert2677 (ImagineerColbert)]

There are many styling tools you have access to, from headings to blockquotes and HTML blocks. It’d be worth experimenting with formatting at some point in time. No forum comes without a set of tools to stylise with; if it does, it should have them.

Not everyone has access to a computer, whether because they don’t own one or aren’t within the vicinity of one. For example, I browse the DevForum often with free time in school and I read lots of threads on my phone. Anyone can view any topic from any device.

---

@ShadokuSan

Yes, it may look a little hard to read when in use. This purpose of these functions is to make things easier to do , not read . IMO it works well enough (at least how I’ve been using it in my own time).

Programming is about both efficiency in the doing and readability in the writing. If neither can be guaranteed, you start falling into holes and leading other newer developers into such holes as well indirectly, especially in an OSS work. Lack of readability in code can impede maintainability, which is a very important aspect to have in your code.

You are capable of ensuring easy to do and easy to read. If you can’t guarantee one or the other, it devalues the resource (subjective) as a production-ready tool and a ground for new developers to work with or from.

@ShadokuSan

What exactly is confusing about how it’s set up?

This is in reference to the entire module, not just ChangeSame. The API feels unintuitive and confusing to use. If it’s specifically targeted at a new developer audience, then you need to write clear code and provide appropriate documentation as well, along with an API that’s readable and easy to follow. The order of parameters for ChangeSame, as well as its name, are just one example.

I don’t want to go off on a tangent but there are already several other things I hold the same view on. There are a few that I can quickly name without picking apart the whole resource:

• _G.new: Unclear as to what new is. Remember that you’re assigning this to the global table. A name that clearly describes what you’re doing would be more appropriate, such as create or NewInstance. This also breaks convention with how the rest of your functions follow PascalCase while this function is in camelCase.

• _G.PN: PN is an obscure name and doesn’t quickly speak to the developer what it’s for without reading documentation, exploring the code or remembering what it does from either of the aforementioned two methods. Two-letter variable names are horrid for readability. Libraries should be clear and readable as they incorporate functionality into the script.

• _G.Rando: Again on readability. Rando is a weird alternative for Random. Variable names should be clear and appropriate on what they’re going to accomplish. For me, even that one missing letter offsets my workflow.

---

Now that I actually have had the time to look through the source code, I have additional feedback that I need to give. There are a few conventions that need to be changed and don’t employ best practices. As a casual use library this may not concern many but as a learning resource it should.

Yielding at the start of the script’s run time.

LocalScripts only run when placed inside a container that supports the running of client-side code, those of which can link back to the player in some way. There is absolutely no need to yield at the beginning of the script’s lifetime and delays when the code adds the functions to _G.

Poor and inconsistent indentation.

This one should go without saying.

Everything from the squared box down should be moved back a tab:

The end is one tab out:

This one isn’t even broken into new lines:

So on. Code writing is majorly preferential but it helps to standardise even your tabbing so other developers exploring your code can see it in a clear and common fashion. That’s to say, tab should only be used for chunks entering scopes, not for new lines or the like. Ends should be placed at the same tab width as the scope opening statements.

Frequent jumps between use of next and pairs in for loops.

This one is self explanatory. The library can’t make a decision on whether to use pairs or next in the code, which butchers its readability. Please choose one or the other. In the case that the indice is not necessary, I recommend using ipairs given the speedup used in the new VM and how it’d be the proper iterator to use.

Lack of error handling.

A lot of this code makes wide assumptions, especially in the realm of the multi-instance operations. These are easily error bound due to instances sporting different classes that can be missing the specified property. This is more of an implementation issue on the developer’s half but I believe that a beginner resource should also handle error cases for them.

The code should make effort to validate what it’s doing before doing it. This is especially the case for ChangeSame which is a relative rollercoaster that’ll be addressed later.

_G.new ignores instancing performance issues.

_G.new is essentially what RbxUtility.Create was, but poorly done. One of the already-major issues I noticed is that if a Parent argument is specified, its setting isn’t deferred until all other properties are set. Parent should always be set last. See this thread:

PSA: Don't use Instance.new() with parent argument

I’ve discovered a pretty bad performance issue in one of top games that has to do with Instance.new and wanted to write about this, since this is not obvious unless you know the system inside out. Tthere are several ways to create a ROBLOX object in Lua: local obj = Instance.new(‘type’); fill obj fields local obj = Instance.new(‘type’, parent); fill obj fields local obj = util.Create(‘type’, { field1 = value1, … }) If you care at all about performance, please only use the first option - I wi…

I have a similar library on my GitHub page, titled Instance2. It follows the convention of being able to instance while specifying properties to be set as well, but in accordance with the above thread the setting of Parent is deferred. It also fixes vanilla Instance.new. You can view the source here:

https://github.com/colbert2677/Roblox-Utilities/blob/master/Instance2.lua

As for final mentions, there’s a weird piece of code that I don’t understand the purpose of. Attempting to work with it throws errors, regardless of how I do it.

if type(i)=='number' then
	v.Parent=c
else
	c[i]=v
end

Apparently if the type of index i is a number (and not the value), it attempts to parent the value to the created instance c. Is this meant to specify children to be added to the table? If so, not only is this not documented anywhere but this is conventionally awkward to allow a table of properties to also specify children to be parented. A different convention or extension function needs to be available here.

Conventions for _G.ChangeSame are unintuitive.

I’m sure I already outlined this earlier.

@colbert2677

Finally, the API for this module is plain confusing and unintuitive, as are the names for some of these functions. For example, the ChangeSame function accepts a data value first, then varargs next for a collection of instances to change then tops that off with a table of what properties to change. It’s a rollercoaster and doesn’t make much sense for use. I can already see awkward conventions like this hurting readability when used.

The order of parameters are as follows:

• Value to be used for each passed property
• List of instances to be changed
• Properties to be changed

I think that this list is jumbled and it’d make more sense to put them in an comfortable order typical of standard convention when scripting, such as passing the items to change first instead.

• List of instances to be changed
• Properties to be changed
• Value to be set

This function feels overall awkward to use so I don’t have many alternate formatting suggestions to offer at the moment.

---

This is all I have for now. I think I spent a little too much time here. The point mainly resides in my first post and this is just meant to clarify some items as a response.