To what extent is modular scripting “too much”

I create my scripts as modular as possible to identify certain issues with my code as easily as possible. However I feel like I may have been making some of my scripts very modular to an extent that it almost looks logically ridiculous and redundant.

local Players = game:GetService(“Players”)

local function SetupPlayerData(Player)
    --setup Player data 
end

local function LoadPlayerData(Player)
    if not Data then
        SetupPlayerData(Player)
    end
end

local function Initialise(Player)
    --initialise Player
    LoadPlayerData(Player)
end

local function OnPlayerAdded(Player)
    if not Initialised then
        Initialise(Player)
    end
end

Players.PlayerAdded:Connect(OnPlayerAdded)

Is there anything wrong with this kind of scripting?

1 Like

Initialise seems pretty pointless if you ask me. All it does is call LoadPlayerData.

Usually functions do more than simply call other functions and that’s it.

4 Likes

Imo, I’d put the if not Data then and if not Initialised then stuff in with the functions themselves - I usually aim for a “every function does one thing” kind of thing and that would play into it.

But I agree with your approach of having separate functions. Like I said before - every function should have one objective, do that, and do nothing else. I’d just merge a few of them because you’re kind of having two functions for one thing here and that’s probably not good either

1 Like

What initialise usually does is create the int value objects for a leaderboard so that if there is data to load, the data can be loaded into the leaderboard.

This is just an example use case for initialise.

If the function is doing something meaningful and more than just that - sure.

@ThomasChabot sums up what I tried saying in a single sentence:

1 Like

There’s nothing actually “wrong” with your scripting. I use modules all the time but depending for what my modular setup is different. I use OOP(Object Orientated Programming) for datastore since it makes it easy to access and edit.

2 Likes

My general feeling is that if a function is only called from one place within the same script, it often doesn’t need to be a function. Functions within a script should be used to consolidate code that is used multiple places, so you have only one copy to maintain. Consider if the extra layers have any real value. If they are just pass-throughs, and aren’t even doing any useful transformation or currying of the arguments, you can probably collapse away some of the layers. There is no right or wrong here, it’s fine to stub things out sort of verbosely and trim it down later to make it more svelte.

There are notable exceptions, of course, like callback functions.

6 Likes

Another key consideration is re-usability. If you plan on using the same function multiple times, this is great:

  • Adapting code becomes easier as everything is handled from one central location
  • Reduces data redundancy
  • Improves readability

If you’re modularising as much code as you can without any purpose (i.e. you definitely won’t be re-using it again) then this will become a time-consuming practise without the addition of reaped benefits.

3 Likes

Your modular scripting method work since there isn’t limit of that :smirk:, if it can help you for scripting you can do but if you think it’s useless you do what you want! But I think that you are using too much function in this Script!:confused: Your script method is way more heavy than any method that I’ve already saw!:neutral_face:
Especially for Initialise() which (in addition!) is linked from OnPlayerAdded!:upside_down_face:
Tip: If you’d like to keep something like this

local function Initialise(Player)
    LoadPlayerData(Player)
end

local function OnPlayerAdded(Player)
    if not Initialised then
        Initialise(Player)
    end
end

You can do that!

local function OnPlayerAdded(Player)
    if not Initialised then
        --Initialise Player
        LoadPlayerData(Player)
    end
end

But if you don’t want, you are free to decide! It’s your Scripts and I’m just giving some ideas :slight_smile:

1 Like

Sounds more like a thread to throw in #development-support:requests-for-code-feedback.

If I’m going to be short and sweet about it though, I don’t really believe in “too modular”. Just know when it’s appropriate to split off into another module or keep expanding your current one.

I sort of just write as I go and hope things turn out right, otherwise I’m also pretty big on making even the smallest of things look pretty (even in a project where I’m the only developer).

2 Likes

My personal opinion is that as long as your scripts are easy to read they are perfectly acceptable.

I always use modules when available. Usually I have one main script to start a main module. This makes it very easy for scripts to communicate with each other since each module will ALWAYS return the cached value. This allows you to create globally available data, and allows you to configure your scripts easier in the future.

Your code is actually very well written in my opinion. As others have said Initialize seems like it isn’t necessary although I actually think it’s a good idea to split up functions pointlessly if it means you could be maintaining an organized script in the future. Splitting them up in this way makes your scripts’ intentions easy to understand and gives you breathing room in the future.

4 Likes

You mean hard code some values assigned to variables in the Modulescript one time when you write the module, which are then available globally to any script that requires the module but are always the same unchangeable values?

How do you create new globally available data as generated by a script or module during game run time? Such as generating one or more tables in script A and then wanting to access (read and write, get and set) those generated table in Script B? Is that what shared should be used for?

In my opinion, you would have to really try to get to the point where your script is “too modular”. For the most part, it comes down to personal preference. As long as it doesn’t affect performance and negatively impact code readability, I wouldn’t worry about it too much.

My general philosophy is to place a block of code into a function if it is any of the following:

  • A semi-complex procedure and can be reused in different places. I say “semi-complex”, because some things would be silly to put in a separate function (e.g. function destroyPart(part) part:destroy() end).
  • A procedure that has a chance of being changed in the future, especially if this procedure is used very often. In this case, it may make sense to make a very simple operation into a function so that you do not have to meticulously modify every script should you change that operation in the future.
  • A block of code that would be overwhelming/unreadable in the surrounding code. Often times code can get so complex that for your brain’s own good it is better to break it up into sections with functions. Then, its more organized and easier to treat each function call more conceptually (i.e. you don’t have to think about how it works, just what it does).

At the end of the day, don’t waste too much brain power on it. As long as it works, performs well, and is readable, and you like it, you’re good.

3 Likes

Nope. Depending on your use case, you don’t have to hard code data. This is often why modules you see will return a table which exposed methods for other scripts, so this scripts can interact with the module and it’s functionality.

The values in a ModuleScript can be changed. The same instance is returned after every subsequent require following the first one. It doesn’t refresh or start a new copy of that script.

Shared is a global variable that accesses a table. If you want to create data that’s available across scripts of the same context, you can expose methods from the ModuleScript which will facilitate reading or writing functions.

1 Like

So, because the same instance of the module is returned, the getter and setter functions can read and write data stored in a table in the module instance … and the date in such a table can change during run time … and be accessed by any script that has required this module?

1 Like

Mm, you got that right. One thing to know though; the server and each client share individual copies of a ModuleScript.

When a module is required on the server for the first time, the return value is cached. Every other server script that requires the ModuleScript will pull that same instance, which is the power of it. When you make changes to this instance (i.e. add something to a table), that saves. This instance can’t be accessed by the client though.

The client, on require, will have it’s own instance of the module cached which other clients and the server cannot access. Then it’s the same cycle over again.

The instance returned on require from a ModuleScript will share the context of the script that required it. Requiring from the server will give it a server context, while doing so from the client will give it a client-sided context.

3 Likes

thanks colbert2677, appreciate the explanation