Ошибки новичков
Добавлено: 08 окт 2017, 16:32Все новички совершают абсолютно одни и те же ошибки, которые я наблюдаю каждый божий день. Не то, чтобы я нетерпим к ним, нет, я довольно толерантен к изъянам в коде (т.к. сам когда-то через это проходил), но вот три ошибки, которые описаны ниже, раздражают меня больше всего.
Сокрытие данных.
Выставляйте наружу как можно меньше полей и свойств. Если необходимо поле или свойство сделать публичным, то ограничивайте уровень доступа у геттера и сеттера. В идеале класс должен представлять из себя запечатанный черный ящик.
Если видимость в инспекторе не нужна, то ограничьте сеттер.
Чтобы сделать поле видимым в инспекторе, примените к нему модификатор доступа private и аттрибут SerializeField.
Таким образом, мы получаем сразу два преимущества: пользователь (например, геймдизайнер) сможет настраивать значение из инспектора + никакой другой класс не сможет изменить свойство SomeVariable напрямую из кода, что в противном случае могло бы нарушить состояние объекта. Представьте сценарий, в котором вы описываете класс с нарушенной инкапсуляцией. В ходе разработки приходит программист Вася и видит, что у вашего класса наружу торчит некое вкусное поле. Вася предполагает, что значение этого поля можно невозбранно менять и, конечно же, меняет его, ломая при этом вашу прекрасную логику, завязанной на значении этого поля. Проще говоря, пользователь (вы или другой программист) класса "Ручка" не должен знать о типе механизма ее открытия. Ручка должна просто писать.
Кэширование компонентов.
Как говорят бородатые разработчики Unity: «Кэшируйте, кэшируйте и кэшируйте!». Рассмотри вымученный искусственный пример:
Каждый раз, чтобы обратиться к компоненту колпачка ручки, мы вызываем GetComponent<BaseCap>(), что в итоге захламляет код, делает его плохо читаемым и медленным. Тут на форуме можно увидеть реальные километровые портянки из GetComponent<T>(). Выход — закэшировать BaseCap.
Согласитесь, что код стал чище и компактнее. К тому же мы освободили ресурсы ЦПУ от постоянного судорожного поиска необходимого нам компонента. Круто? Ну, не очень. Скорее всего в реальной жизни вашему компоненту будет жизненно необходим другой, без которого он попросту не сможет работать. Поэтому можно воспользоваться аттрибутом RequireComponent и избавиться от проверки на null.
Лирическое отступление.
Теперь немного порассуждаем. Предположим, моделлер выгрузил ручку с колпачком, да так, что сама меша колпачка находится где-то в жопе по иерархии. К тому же как-то странно вешать ̶м̶о̶д̶е̶л̶л̶е̶р̶а̶ компонент колпачка на тот же объект, где висит компонент самой ручки. Логичнее повесить компонент колпачка на сам колпачок, а в инспекторе в компоненте ручки сделать поле для ссылки на колпачок. Тогда код упрощается еще больше.
Может возникнуть вопрос: а почему не воспользоваться GetComponentInChildren<BaseCap>()? Пожалуйста, если вы уверены, что колпачки будут всегда находиться в иерархии ручки.
Однако, как подсказывает практика, завтра обязательно решат, что все колпачки будут идти отдельно, и вообще мы будем их продавать через встроенный магазин за биткоины.
Работа в Update.
Люди очень часто выносят тяжелую логику в Update, не понимая, что все тоже самое можно сделать гораздо «легковеснее», если использовать событийно-ориентированный подход.
Во-первых, компоненты UI ничего не должны знать о персонаже! За такой код вас нужно гнать ссаными тряпками. Во-вторых, зачем КАЖДЫЙ РАЗ в Update переприсваивать значение здоровья персонажа, вызывая при этом тяжелейший string.Format() и ToString(), когда это можно сделать ОДИН РАЗ при ИЗМЕНЕНИИ здоровья?
Если игрока ранили или он подобрал аптечку, вызывается событие изменения здоровья, на которое подписан MainController. MainController знает об UI и вызывает у него необходимый метод для обновления. Все! И никакого обновления 120 раз в секунду.
Приводите свои примеры для улучшения качества кода!
Сокрытие данных.
Выставляйте наружу как можно меньше полей и свойств. Если необходимо поле или свойство сделать публичным, то ограничивайте уровень доступа у геттера и сеттера. В идеале класс должен представлять из себя запечатанный черный ящик.
Синтаксис:
Используется csharp
public class SomeClass : MonoBehaviour
{
public int SomeVariable;
}
{
public int SomeVariable;
}
Если видимость в инспекторе не нужна, то ограничьте сеттер.
Синтаксис:
Используется csharp
namespace SomeClasses
{
public class SomeClass : MonoBehaviour
{
public int SomeVariable
{
get;
private set;
}
}
}
{
public class SomeClass : MonoBehaviour
{
public int SomeVariable
{
get;
private set;
}
}
}
Чтобы сделать поле видимым в инспекторе, примените к нему модификатор доступа private и аттрибут SerializeField.
Синтаксис:
Используется csharp
namespace SomeClasses
{
public class SomeClass : MonoBehaviour
{
[SerializeField]
private int someVariable;
public int SomeVariable
{
get
{
return someVariable;
}
}
}
}
{
public class SomeClass : MonoBehaviour
{
[SerializeField]
private int someVariable;
public int SomeVariable
{
get
{
return someVariable;
}
}
}
}
Таким образом, мы получаем сразу два преимущества: пользователь (например, геймдизайнер) сможет настраивать значение из инспектора + никакой другой класс не сможет изменить свойство SomeVariable напрямую из кода, что в противном случае могло бы нарушить состояние объекта. Представьте сценарий, в котором вы описываете класс с нарушенной инкапсуляцией. В ходе разработки приходит программист Вася и видит, что у вашего класса наружу торчит некое вкусное поле. Вася предполагает, что значение этого поля можно невозбранно менять и, конечно же, меняет его, ломая при этом вашу прекрасную логику, завязанной на значении этого поля. Проще говоря, пользователь (вы или другой программист) класса "Ручка" не должен знать о типе механизма ее открытия. Ручка должна просто писать.
Кэширование компонентов.
Как говорят бородатые разработчики Unity: «Кэшируйте, кэшируйте и кэшируйте!». Рассмотри вымученный искусственный пример:
Синтаксис:
Используется csharp
namespace Pens
{
public class BasePen : MonoBehaviour
{
public void Close ()
{
/* Каждый раз, когда вы вызываете GetComponent, где-то
в мире умирает котенок */
if (GetComponent<BaseCap> () != null)
{
GetComponent<BaseCap> ().Install ();
}
}
public void Draw ()
{
bool canWrite = true;
if (GetComponent<BaseCap> () != null)
canWrite = !GetComponent<BaseCap> ()).Installed;
if (canWrite)
{
//Много сложного кода
}
else
{
//Бросаем исключение/взрываем компуктер/ничего не делаем
}
}
}
}
{
public class BasePen : MonoBehaviour
{
public void Close ()
{
/* Каждый раз, когда вы вызываете GetComponent, где-то
в мире умирает котенок */
if (GetComponent<BaseCap> () != null)
{
GetComponent<BaseCap> ().Install ();
}
}
public void Draw ()
{
bool canWrite = true;
if (GetComponent<BaseCap> () != null)
canWrite = !GetComponent<BaseCap> ()).Installed;
if (canWrite)
{
//Много сложного кода
}
else
{
//Бросаем исключение/взрываем компуктер/ничего не делаем
}
}
}
}
Каждый раз, чтобы обратиться к компоненту колпачка ручки, мы вызываем GetComponent<BaseCap>(), что в итоге захламляет код, делает его плохо читаемым и медленным. Тут на форуме можно увидеть реальные километровые портянки из GetComponent<T>(). Выход — закэшировать BaseCap.
Синтаксис:
Используется csharp
namespace Pens
{
public class BasePen : MonoBehaviour
{
private BaseCap cap;
public void Close ()
{
if (cap != null)
cap.Install ();
}
public void Draw ()
{
bool canWrite = true;
if (cap != null)
canWrite = !cap.Installed;
if (canWrite)
{
//Много сложного кода
}
else
{
//Бросаем исключение/взрываем компуктер/ничего не делаем
}
}
void Start ()
{
cap = GetComponent<BaseCap> ();
}
}
}
{
public class BasePen : MonoBehaviour
{
private BaseCap cap;
public void Close ()
{
if (cap != null)
cap.Install ();
}
public void Draw ()
{
bool canWrite = true;
if (cap != null)
canWrite = !cap.Installed;
if (canWrite)
{
//Много сложного кода
}
else
{
//Бросаем исключение/взрываем компуктер/ничего не делаем
}
}
void Start ()
{
cap = GetComponent<BaseCap> ();
}
}
}
Согласитесь, что код стал чище и компактнее. К тому же мы освободили ресурсы ЦПУ от постоянного судорожного поиска необходимого нам компонента. Круто? Ну, не очень. Скорее всего в реальной жизни вашему компоненту будет жизненно необходим другой, без которого он попросту не сможет работать. Поэтому можно воспользоваться аттрибутом RequireComponent и избавиться от проверки на null.
Синтаксис:
Используется csharp
[RequireComponent (typeof (BaseCap))]
namespace Pens
{
public class BasePen : MonoBehaviour
{
private BaseCap cap;
public void Close ()
{
cap.Install ();
}
public void Draw ()
{
if (!cap.Installed)
{
//Много сложного кода
}
else
{
//Бросаем исключение/взрываем компуктер/ничего не делаем
}
}
void Start ()
{
cap = GetComponent<BaseCap> ();
}
}
}
namespace Pens
{
public class BasePen : MonoBehaviour
{
private BaseCap cap;
public void Close ()
{
cap.Install ();
}
public void Draw ()
{
if (!cap.Installed)
{
//Много сложного кода
}
else
{
//Бросаем исключение/взрываем компуктер/ничего не делаем
}
}
void Start ()
{
cap = GetComponent<BaseCap> ();
}
}
}
Лирическое отступление.
Теперь немного порассуждаем. Предположим, моделлер выгрузил ручку с колпачком, да так, что сама меша колпачка находится где-то в жопе по иерархии. К тому же как-то странно вешать ̶м̶о̶д̶е̶л̶л̶е̶р̶а̶ компонент колпачка на тот же объект, где висит компонент самой ручки. Логичнее повесить компонент колпачка на сам колпачок, а в инспекторе в компоненте ручки сделать поле для ссылки на колпачок. Тогда код упрощается еще больше.
Синтаксис:
Используется csharp
namespace Pens
{
public class BasePen : MonoBehaviour
{
[SerializeField]
private BaseCap cap;
public void Close ()
{
cap.Install ();
}
public void Draw ()
{
if (!cap.Installed)
{
//Много сложного кода
}
else
{
//Бросаем исключение/взрываем компуктер/ничего не делаем
}
}
}
}
{
public class BasePen : MonoBehaviour
{
[SerializeField]
private BaseCap cap;
public void Close ()
{
cap.Install ();
}
public void Draw ()
{
if (!cap.Installed)
{
//Много сложного кода
}
else
{
//Бросаем исключение/взрываем компуктер/ничего не делаем
}
}
}
}
Может возникнуть вопрос: а почему не воспользоваться GetComponentInChildren<BaseCap>()? Пожалуйста, если вы уверены, что колпачки будут всегда находиться в иерархии ручки.
Однако, как подсказывает практика, завтра обязательно решат, что все колпачки будут идти отдельно, и вообще мы будем их продавать через встроенный магазин за биткоины.
Работа в Update.
Люди очень часто выносят тяжелую логику в Update, не понимая, что все тоже самое можно сделать гораздо «легковеснее», если использовать событийно-ориентированный подход.
Синтаксис:
Используется csharp
namespace MySuperGame.UI
{
public class Health : MonoBehaviour
{
[SerializeField]
private Text text;
private Player player;
void Start ()
{
player = FindObjectOfType<Player> ();
}
void Update ()
{
text.text = string.Format ("Жыза: {0}%", player.Health.ToString ());
}
}
}
{
public class Health : MonoBehaviour
{
[SerializeField]
private Text text;
private Player player;
void Start ()
{
player = FindObjectOfType<Player> ();
}
void Update ()
{
text.text = string.Format ("Жыза: {0}%", player.Health.ToString ());
}
}
}
Во-первых, компоненты UI ничего не должны знать о персонаже! За такой код вас нужно гнать ссаными тряпками. Во-вторых, зачем КАЖДЫЙ РАЗ в Update переприсваивать значение здоровья персонажа, вызывая при этом тяжелейший string.Format() и ToString(), когда это можно сделать ОДИН РАЗ при ИЗМЕНЕНИИ здоровья?
Синтаксис:
Используется csharp
namespace MySuperGame.UI
{
/// <summary>
/// Класс, отвечающий за отображение здоровья персонажа
/// </summary>
public class Health : MonoBehaviour
{
private const string HEALTH_FORMAT = "Жыза: {0}%";
[SerializeField]
private Text text;
/// <summary>
/// Обновляет значение здоровья персонажа
/// </summary>
/// <param name="value">Значение здоровья</param>
public void Set (float value)
{
text.text = string.Format (HEALTH_FORMAT, value.ToString ());
}
}
}
namespace MySuperGame
{
public class Player : MonoBehaviour
{
/// <summary>
/// Максимальное значение здоровья игрока
/// </summary>
private const float MAX_HEALTH = 100f;
/// <summary>
/// Событие, возникающее при изменении здоровья
/// </summary>
public event System.Action<Player> OnHealthChanged;
private void DoHealthChanged ()
{
if (OnHealthChanged != null)
{
OnHealthChanged (this);
}
}
private float health;
/// <summary>
/// Значение здоровья персонажа
/// </summary>
public float Health
{
get
{
return health;
}
set
{
float _health = Mathf.Clamp (value, 0f, MAX_HEALTH);
/* если предыдущее значение здоровья не равняется текущему */
if (health != _health)
{
/* то присваимваем новое значение здоровья */
health = _health;
/* вызываем событие изменения здоровья */
DoHealthChanged ();
}
}
}
}
/// <summary>
/// Класс, реализующий логику игры
/// </summary>
public class MainController : MonoBehaviour
{
[SerializeField]
private Player player;
[SerializeField]
private Health health;
private void Player_OnHealthChanged (Player sender)
{
/* обновляем значение здоровья в UI */
health.Set (sender.Health);
}
void Start ()
{
/* подписываемся на событие изменения здорвоья персонажа */
player.OnHealthChanged += Player_OnHealthChanged;
}
}
}
{
/// <summary>
/// Класс, отвечающий за отображение здоровья персонажа
/// </summary>
public class Health : MonoBehaviour
{
private const string HEALTH_FORMAT = "Жыза: {0}%";
[SerializeField]
private Text text;
/// <summary>
/// Обновляет значение здоровья персонажа
/// </summary>
/// <param name="value">Значение здоровья</param>
public void Set (float value)
{
text.text = string.Format (HEALTH_FORMAT, value.ToString ());
}
}
}
namespace MySuperGame
{
public class Player : MonoBehaviour
{
/// <summary>
/// Максимальное значение здоровья игрока
/// </summary>
private const float MAX_HEALTH = 100f;
/// <summary>
/// Событие, возникающее при изменении здоровья
/// </summary>
public event System.Action<Player> OnHealthChanged;
private void DoHealthChanged ()
{
if (OnHealthChanged != null)
{
OnHealthChanged (this);
}
}
private float health;
/// <summary>
/// Значение здоровья персонажа
/// </summary>
public float Health
{
get
{
return health;
}
set
{
float _health = Mathf.Clamp (value, 0f, MAX_HEALTH);
/* если предыдущее значение здоровья не равняется текущему */
if (health != _health)
{
/* то присваимваем новое значение здоровья */
health = _health;
/* вызываем событие изменения здоровья */
DoHealthChanged ();
}
}
}
}
/// <summary>
/// Класс, реализующий логику игры
/// </summary>
public class MainController : MonoBehaviour
{
[SerializeField]
private Player player;
[SerializeField]
private Health health;
private void Player_OnHealthChanged (Player sender)
{
/* обновляем значение здоровья в UI */
health.Set (sender.Health);
}
void Start ()
{
/* подписываемся на событие изменения здорвоья персонажа */
player.OnHealthChanged += Player_OnHealthChanged;
}
}
}
Если игрока ранили или он подобрал аптечку, вызывается событие изменения здоровья, на которое подписан MainController. MainController знает об UI и вызывает у него необходимый метод для обновления. Все! И никакого обновления 120 раз в секунду.
Приводите свои примеры для улучшения качества кода!