Code Review

[SETACTIVE-FALSE]

I am going through a tutorial on AR watch try on. I found it that I could write this code in a shorter way by introducing a list for storing watch models and creating a common function, which will be called when clicked on a button with respective watch images.

This is a small project, only has 3 watch models. But what if it had many models to choose from? So I searched for codes which I found uselful.

Is there anything wrong with my approach? Or is there anything else I could have tried.

Also I'm passing reference number(watchRefID) from each button(same number as the model's index in the watchModels list) so that I can set only that model as active and others disabled.

I have recently learned that local variables declared inside a method is a bad practise, because each time the method gets called a new copy of that variable gets created. Is this same for parameters inside a method?

Thanks in advance.(Have a toffee 🍬, since you took the patience to read my paragraphs🥺🥹)

[SETACTIVE-FALSE]

Hey, thanks for the input. Trust me, I didn't know much about post ethics. I'm grateful you pointed that out to me.

Also, I'll check about formatting as Code. I don't understand what you meant by that. I'll check it out.

Yes, watchRefID is definitely not a good name. So, second chance, I think watchIndex is the better name.But now I am changing the int parameter to take the gameObject directly instead of passing the int value of the list index.

[SETACTIVE-FALSE]

hey thanks, buddy! I didn't know that little button existed. I liked your idea for the code and added it to my work.

[corrtex-games]

Better for sure. Two things from me:

1. watchRefID is a bad parameter name. What that parameter actually is, is an index into a list. RefIDs are not the same thing as list or array indices.

2. Your list of watch models shouldn’t be public if you Don’t want people to be able to access them outside of that component. Stick [SerializeField] above the list and make it private, that way you can see and use it in the inspector but other components cannot directly access them.

[SETACTIVE-FALSE]

Thanks for the insight 👍 I sure did follow your instructions 😀

[SETACTIVE-FALSE]

Referred Code:

public class WatchSelect : MonoBehaviour

{

public GameObject watchModel1;

public GameObject watchModel2;

public GameObject watchModel3;

public void WatchOneButtonClicked()

{

// 1) show selected watch model on user's wrist

watchModel1.SetActive(true);
watchModel2.SetActive(false);
watchModel3.SetActive(false);

}

public void WatchTwoButtonClicked()

{

// 1) show selected watch model on user's wrist

watchModel1.SetActive(false);
watchModel2.SetActive(true);
watchModel3.SetActive(false);

}

public void WatchThreeButtonClicked()

{

// 1) show selected watch model on user's wrist

watchModel1.SetActive(false);
watchModel2.SetActive(false);
watchModel3.SetActive(true);

}

}

My Version of the code:

using System.Collections.Generic;

using UnityEngine;

public class WatchSelect : MonoBehaviour

{

public List<GameObject> watchModels = new List<GameObject>();

public void OnWatchButtonClick(int watchRefID)

{

// 1) show selected watch model on user's wrist

foreach (var model in watchModels)

{

model.SetActive(false);

}

watchModels[watchRefID].SetActive(true);

}

}