Помогите исправить код на более читабельнее
создал проект Windows Forms, хочу красиво задизайнить но так, чтобы и код не был этим "засран".
public partial class FormMain : Form
{
private Button currentButton;
private Form currentForm;
private ImageList currentImageList;
public FormMain()
{
InitializeComponent();
}
private void ActivateButton(Button button, Form form, ImageList imageList)
{
if (button != null)
{
DisableButton();
currentForm = form;
currentImageList = imageList;
currentButton = button;
if (imageList != null)
currentButton.BackgroundImage = currentImageList.Images[1];
panelMain.Controls.Add(currentForm);
}
}
private void DisableButton()
{
if (currentButton != null)
{
if (currentImageList != null)
currentButton.BackgroundImage = currentImageList.Images[0];
}
panelMain.Controls.Remove(currentForm);
}
private void ButtonFavoriteList_Click(object sender, EventArgs e)
{
ActivateButton(buttonFavoriteList, new FormFavoritesList(), imageFavoriteList);
}
Ответы (1 шт):
Исходя из общих соображений, раз вы не указываете что не нравится и зачем что-то переделывать. Понятие читабельности и красоты оно же у многих разное.
В этом методе
private void ActivateButton(Button button, Form form, ImageList imageList)
{
if (button != null)
{
DisableButton();
currentForm = form;
currentImageList = imageList;
currentButton = button;
if (imageList != null)
currentButton.BackgroundImage = currentImageList.Images[1];
panelMain.Controls.Add(currentForm);
}
}
можно инвертировать if:
private void ActivateButton(Button button, Form form, ImageList imageList)
{
if (button == null)
return;
DisableButton();
currentForm = form;
currentImageList = imageList;
currentButton = button;
if (imageList != null)
currentButton.BackgroundImage = currentImageList.Images[1];
panelMain.Controls.Add(currentForm);
}
Этот приём экономит место по горизонтали, в случае когда у вас начинает вглубь идти код лесенкой код не влезает по горизонтали.
При этом неясно, отчего такая неуверенность, что вы постоянно спохватываетесь, что button может быть null. Может стоит один раз проверить в одном месте и дальше спокойно передавать в расчёте что там != null?
Непонятно, почему эти два if не объединены в один:
private void DisableButton()
{
if (currentButton != null)
{
if (currentImageList != null)
currentButton.BackgroundImage = currentImageList.Images[0];
}
panelMain.Controls.Remove(currentForm);
}
Очевидно:
private void DisableButton()
{
if (currentButton != null && currentImageList != null)
{
currentButton.BackgroundImage = currentImageList.Images[0];
}
panelMain.Controls.Remove(currentForm);
}
Вообще же напрашивается переход на null conditional operator, его ещё в C#6 завезли. (И ещё снова о том, что можно разок проверить button на null)
Непонятно зачем вам так нужна текущая кнопка:
private Button currentButton;
Убираем, передаём в метод:
DisableButton(button);
private void DisableButton(Button button) { ... }
Также у вас то одна пустая строка между методами, то ни одной -- но вам это видимо глаз не мозолит, поэтому я делаю вывод, что код вы форматируете руками, а не какими-то инструментами типа Resharper/StyleCop, а зря. Во-первых, они огромное количество рутинной работы за вас сделают, а во-вторых, вы сразу научитесь видеть небрежность и неаккуратность.
Как-то так на первый взгляд не разбираясь в логике того, что есть. Код вы привели не полностью, так что непонятно, сколько каких мест может вызвать эти методы, где могут быть null вместо кнопки, зачем вам эти поля и т.п.