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

Question

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

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

EG:

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"];
}

Answer 1

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

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

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

Answer 2

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

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

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

Answer 3

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