koobas.hobune.stream
Is it worth making tons of similiar methods or create big one that connects things together?
My game uses 2 'local' currencies that are set to 0 on game start and you gain them in game, and when you finish the level they are added to 2 'global' currencies that work around in menu, can be used in shop, the global currency is also loaded from save on game load, etc. should I use methods like
AddLocalCoins(int), AddLocalGems(int), AddGlobalCoins(int), AddGlobalGems(int), DeductX(int) (for these 4)
and similiar, or just create one method that will support all of these things? Like:
public void AddCollectibles(int coinsOrGems, int localOrGlobal, int amountToAdd)
{
if (coinsOrGems == 0)
{
if (localOrGlobal == 0) { _localCoins += amountToAdd; }
else { _globalCoins += amountToAdd; }
UpdateCoinAmount();
}
else
{
if (localOrGlobal == 0) { _localGems += amountToAdd; }
else { _globalGems += amountToAdd; }
UpdateGemAmount();
}
}
Wouldn't separate UpdateCoinAmount()/Gem methods be needed too? (they update currency texts based on the amount, local and global text gameobjects are different). Do you use separate methods for Add and Subtract, considering you can add negative values? What is the correct practice for that? Where's the limit?
First, separate the responsibilities of your code as much as possible: if you have a single method that could be split into 2, 3, or even more independent but communicating methods, it's a great improvement.
Second, make your methods as parametrizable as possible: if you use a 1 anywhere in your code, don't hard-code it: pass that 1 as a parameter. This way the same method can be easily used to solve other needs.
So to answer your question:
public void AddCollectibles(int coinsOrGems, int localOrGlobal, int amountToAdd)
Yes, making your methods parametrizable like this is good. There are many ways you could make your code more streamlined and reusable too. Say, instead of storing coins and gems in multiple different variables, you could store them in a multidimensional array.
Then you could use the parameters received by the method to access the correct data in a single line instead of stacking if conditions.
Like so:
//collectible storage defined as an array
//first dimension separates coins and gems, second dimension separates local and global
int[,] collectibleStorage = new int[2,2];
public void AddCollectibles(int coinsOrGems, int localOrGlobal, int amountToAdd)
{
//local coins will be stored in [0, 0], global coins will be stored in [0, 1]
//local gems will be stored in [1, 0], global gems will be stored in [1, 1]
collectibleStorage[coinsOrGems, localOrGlobal] += amountToAdd;
if (coinsOrGems == 0) { UpdateCoinAmount(); }
else { UpdateGemAmount(); }
}
Thanks, the multidimensional array solution looks smart, however it's sad that unity won't show it with serializefield, my currency script is still in the works and wanted to see if the things get calculated correctly. There are solutions like using array of vector2's or using classes/structs, and I don't understand the later that much but I think they would be overkill, so let's just hope the currency will work okay as multidimensional array for now.
What do you think about the add + subtract methods? I still have DeductGlobalCoin(amount) used for the shops and the big public void AddCollectibles(int coinsOrGems, int localOrGlobal, int amountToAdd)used for the rest. Should I think about redoing the shop so the prices are negative and just += negative values? That also always bothers me when I make damage (TakeHit) scripts.
I'd take care to keep the array logic, which index means what, in a single place. For example rename AddCollectibles -> $$anonymous$$odifyCollectibles (or even AddOrDeductCollectibles), and then just call it with positive or negative values. I might still do DeductGlobalCoin (like in my version below) etc to make it clear and foolproof to use, to not miss a - sign accidentally somewhere. And I've always kept prices and damage as positive for the items and then have had the $$anonymous$$us logic in code.
Damn, it's getting me a bit overwhelmed I'm thinking about adding add/deduct/set parameter to the method. The reason for that is
I display price tags in shop dynamically, they just get int of the price and set the text based on that, I would have to set the text with absolute value of negative price or remove the $$anonymous$$us from the text.
I could simply deduct the positive values within single method
I hoped that with my save/load serialized system the adding of global collectibles on data load would work fine, but it's creating glitches because I load the data everytime I finish the game and add local currency on top of that, with my current structure I need to set it, not add.
However I am starting to get afraid that adding so many parameters may create some kind of overload as it is mobile game and I would call the method on every enemy kill.
Native arrays of serializable types (like an int) CAN BE serialized.
Remember that for a field to be serialized it must be public scoped, like so:
[SerializeField]
public int[,] collectibleStorage = new int[2,2];
private fields (without a public identifier) can never be serialized Serializable types list
That's true only for one dimension arrays, as soon as I add the [,] and ,2 the array disappears from inspector after compile, I mean it may work in background but I guess custom inspector would be needed to show that
It's often good to make several methods for the things you need. But then have common infrastructure so that you don't need to repeat the same things in many places. @mephistoII666 has a good answer here, but my take on the code is a bit different.
The method in the question is very bad, like an example of how you should not bundle two different methods in one and then have a parameter and if-else to choose which one to run.
Anyway in this case you can make a single method to do this - like @mephistoII666 's AddCollectibles above, or my AddCollectible below.
It can be useful though then to provide a public interface for doing what you want with simple calls from the outside. It's a bit of a matter of taste -- also with my solution, you can also just skip those little helper methods and have users call AddCollectible directly. It can be nice however that the internals of the class don't need to be known by outside users of the class.
You tell how usage of local and global counts are different, so I figured it's clearest to have the data for them separate. Also I like named indices instead of 0 or 1 for the type. This might be overkill for this simple thing but I like the clarity of enums and Dictionaries.
using System;
using System.Collections.Generic;
class MainClass {
public static void Main (string[] args) {
Inventory inv = new Inventory();
inv.AddCoinsLocal(1);
inv.AddCoinsGlobal(2);
inv.AddGemsLocal(3);
inv.AddGemsGlobal(4);
inv.Show();
}
}
enum Collectible
{
Coin,
Gem
}
class Inventory
{
Dictionary<Collectible, int> local;
Dictionary<Collectible, int> global;
public Inventory()
{
local = new Dictionary<Collectible, int>();
global = new Dictionary<Collectible, int>();
local[Collectible.Coin] = 0;
global[Collectible.Coin] = 0;
local[Collectible.Gem] = 0;
global[Collectible.Gem] = 0;
}
public void AddCoinsLocal(int amount)
{
AddCollectible(local, Collectible.Coin, amount);
}
public void AddCoinsGlobal(int amount)
{
AddCollectible(global, Collectible.Coin, amount);
}
public void AddGemsLocal(int amount)
{
AddCollectible(local, Collectible.Gem, amount);
}
public void AddGemsGlobal(int amount)
{
AddCollectible(global, Collectible.Gem, amount);
}
private void AddCollectible(Dictionary<Collectible, int> localOrGlobal, Collectible collectibleType, int amount)
{
localOrGlobal[collectibleType] += amount;
}
public void Show()
{
foreach (int amount in local.Values)
Console.WriteLine(amount);
foreach (int amount in global.Values)
Console.WriteLine(amount);
}
}
Executable in replit at: https://repl.it/@ToniAlatalo/SameTimelyRule
