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

Иногда я допускаю глубокие нюансы ключевого слова 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.