Рефакторинг большого шара грязи; не уверен, что статичность здесь используется правильно. Совет?

Иногда я допускаю глубокие нюансы ключевого слова static.

Вот что я вижу:

public partial class Default : CSSDEIStatusBase
{ private static Default _csWitWeb; protected void Page_Load(object sender, EventArgs e) { //DoStuff _csWitWeb = this; //OtherStuff } public static void ForceLoadSyncOperation(int? index) { Default._csWitWeb.LoadSelectedSyncOperation(index); }
}

Единственными ссылками на ForceLoadSyncOperation являются:

Default.ForceLoadSyncOperation(index);

или

Default.ForceLoadSyncOperation(null);

Оба этих вызова происходят из:

public partial class DataOriginUserControl : System.Web.UI.UserControl

и не находятся внутри статических методов.

НАПРИМЕР:

protected void btnCancelSyncOperation_Click(object sender, EventArgs e)
{ this.lblErrorMessage.Text = string.Empty; this.lblErrorMessage.Visible = false; int index = _syncOperation.Sequence - 1; Default.ForceLoadSyncOperation(index);
}

Все это кажется мне очень причудливым. Этот запах кому-то еще? Не совсем уверен, как распутать его, хотя я не могу точно создать экземпляр страницы Default внутри пользовательского элемента управления.

Мысли? Спасибо за прочтение.

protected void LoadSelectedSyncOperation(int? index)
{ SyncOperationConfiguration[] syncOperations = CSServiceClient.GetInterfaceConfiguration().SyncOperationConfigurations.ToArray(); PopulateSyncOperationsListView(syncOperations); SyncOperationConfiguration syncOperation = null; try { syncOperation = syncOperations[index.HasValue ? index.Value : 0]; } catch { syncOperation = syncOperations[0]; } ucDataOrigin.LoadSyncOperationData(syncOperation); Session["ConfigMenuActiveIndex"] = 1; menuConfiguration.Items[(int)Session["ConfigMenuActiveIndex"]].Selected = true; mvwConfiguration.ActiveViewIndex = (int)Session["ConfigMenuActiveIndex"];
}
3 ответа

Предположительно, пользовательский элемент управления содержится на странице " Default а статический член используется в качестве ярлыка для получения текущего экземпляра " Default. Я бы сделал это так:

Default defaultPage = this.Page as Default;
if (defaultPage != null)
{ defaultPage.LoadSelectedSyncOperation(index);
}

Использование статического члена таким образом небезопасно. Он открывает дверь для условий гонки. Существует потенциальный риск того, что пользовательский элемент управления загружается на другую страницу и вызывает LoadSelectedSyncOperation() в отдельном экземпляре запроса Default, тем самым разрушая всевозможные потенциальные хаосы.


Я не знаю, что делает LoadSelectedSyncOperation, но этот код выглядит странно. Всякий раз, когда вы нажимаете btnCancelSyncOperation, вы вызываете этот метод на какой-либо странице, но никогда не знаете, на каком из них. Это не имеет большого значения для меня.


Я бы определенно сказал, что ваши опасения действительны. Я не могу думать ни о какой причине, что этот дизайн имел бы смысл. Это тоже поднимет мне флаг.

Основываясь на ответе на мой комментарий, если Default.LoadSelectedSyncOperation каким-то образом не зависит от страницы по умолчанию, то я предлагаю его реорганизовать в отдельный класс (а не страницу ASP.NET).

Имеет ли смысл, чтобы метод или новый класс был статичным или нет, является отдельной проблемой и будет основываться на логике, содержащейся в этом методе.

licensed under cc by-sa 3.0 with attribution.