4
\$\begingroup\$

This question provides the code behind the dialogs in the C# refactoring question. I'd like a review of this code as well. I'm not currently using databinding anywhere, and there is really only one place that it might help which is in the add or edit tenant form. This question is separate from the other question because there is too much code to present in one question.

Visual Studio generated code is not included in this question.

Features

  • Generate MS Word documents from the data in the Excel spreadsheet for use as directories by mailboxes.
  • Add or Edit a tenant to the complex.
  • Delete a tenant from the complex.
  • Set preferences for when to save or print a file. Where to store saved Word documents. Where to find the Excel spreadsheet file.
  • Save any current changes.
  • The data is saved automatically when the program exits.
  • If the Excel file is open in another application it warns the user and will not start. If the user open it in Excel after the program starts it warns them that the changes can't be saved until the Excel file is closed.

RentRosterApp Form, the Control Panel Print Mailbox List Dialog Add or Edit Tenant Dialog Delete Tenant Dialog Edit Preferences Dialog

RentRosterApp.cs

This is the main form started by the Program.cs module, consider it a control panel, it only contains buttons that open other dialogs. It does however contain some program logic for global initialization.

using System;
using System.Drawing;
using System.Windows.Forms;

namespace TenantRosterAutomation
{
    public partial class RentRosterApp : Form
    {
        private bool globalsInitialized;

        public RentRosterApp()
        {
            globalsInitialized = Globals.InitializeAllModels();
            InitializeComponent();
        }

        private void RentRosterApp_Load(object sender, EventArgs e)
        {
            RR_Quit_BTN.BackColor = Color.Red;
            if (!globalsInitialized)
            {
                PrintMailboxLists_Button.Enabled = false;
                AddNewResident_Button.Enabled = false;
                DeleteRenter_Button.Enabled = false;

                using (EditPreferencesDlg preferences_dlg = new EditPreferencesDlg())
                {
                    if (preferences_dlg.ShowDialog() == DialogResult.OK)
                    {
                        PrintMailboxLists_Button.Enabled = true;
                        AddNewResident_Button.Enabled = true;
                        DeleteRenter_Button.Enabled = true;
                    }
                }
            }
        }

        private void PrintMailboxLists_Button_Click(object sender, EventArgs e)
        {
            PrintMailboxListsDlg printMailboxLists_dialog = new PrintMailboxListsDlg();
            printMailboxLists_dialog.Show();
        }

        private void AddNewResident_Button_Click(object sender, EventArgs e)
        {
            ApartmentNumberVerifier verifier_Form = new ApartmentNumberVerifier();
            verifier_Form.NextAction = ApartmentNumberVerifier.NextActionEnum.ADD;
            verifier_Form.Show();
        }

        private void DeleteRenter_Button_Click(object sender, EventArgs e)
        {
            ApartmentNumberVerifier verifier_Form = new ApartmentNumberVerifier();
            verifier_Form.NextAction = ApartmentNumberVerifier.NextActionEnum.DELETE;
            verifier_Form.Show();
        }

        private void EditPreferences_BTN_Click(object sender, EventArgs e)
        {
            EditPreferencesDlg preferences_dlg = new EditPreferencesDlg();
            preferences_dlg.Show();
        }

        private void RR_Quit_BTN_Click(object sender, EventArgs e)
        {
            try
            {
                Globals.Save();
                Globals.ReleaseAllModels();
                Close();
            }
            catch (AlreadyOpenInExcelException ao)
            {
                MessageBox.Show(ao.Message);
            }
        }

        private void RR_SAVEEDITS_BTN_Click(object sender, EventArgs e)
        {
            try
            {
                Globals.SaveTenantData();
            }
            catch (AlreadyOpenInExcelException ao)
            {
                MessageBox.Show(ao.Message);
            }
        }

        private void RentRosterApp_FormClosing(object sender, FormClosingEventArgs e)
        {
            // Save all changes before quiting.
            Globals.Save();
            Globals.ReleaseAllModels();
        }
    }
}

EditPreferencesDlg.cs

This form allows the users to edit their preferences, it is called with a button on the main control panel, but if the preferences have never been created before then the control panel opens it automatically. There might be a fairly strong coupling between this class and the UserPreferences class.

using System;
using System.Collections.Generic;
using System.Drawing;
using System.Windows.Forms;

namespace TenantRosterAutomation
{
    public partial class EditPreferencesDlg : Form
    {
        private UserPreferences localPreferences;

        public EditPreferencesDlg()
        {
            localPreferences = new UserPreferences();
            localPreferences.CopyValues(Globals.Preferences, true);
            InitializeComponent();
        }

        private void EditPreferencesDlg_Load(object sender, EventArgs e)
        {
            EP_Cancel_BTN.BackColor = Color.Red;
            EP_SavePreferences_BTN.BackColor = Color.Green;
            EP_DefaultFileFolder_TB.Text = localPreferences.DefaultSaveDirectory;
            EP_RentRosterLocation_TB.Text = localPreferences.ExcelWorkBookFullFileSpec;
            EP_SheetName_TB.Text = localPreferences.ExcelWorkSheetName;
            switch (localPreferences.PrintSaveOptions)
            {
                case PrintSavePreference.PrintSave.PrintAndSave:
                    EP_PrintAndSave_RB.Checked = true;
                    break;
                case PrintSavePreference.PrintSave.SaveOnly:
                    EP_SavelOnly_RB.Checked = true;
                    break;
                default:
                    EP_PrintOnly_RB.Checked = true;
                    break;
            }
            EP_SheetName_TB.Enabled = false;
            EP_RentRosterSheetName_LISTBOX.Visible = false;
        }

        private void EP_PrintAndSave_RB_CheckedChanged(object sender, EventArgs e)
        {
            localPreferences.PrintSaveOptions = PrintSavePreference.PrintSave.PrintAndSave;
        }

        private void EP_SavelOnly_RB_CheckedChanged(object sender, EventArgs e)
        {
            localPreferences.PrintSaveOptions = PrintSavePreference.PrintSave.SaveOnly;
        }

        private void EP_PrintOnly_RB_CheckedChanged(object sender, EventArgs e)
        {
            localPreferences.PrintSaveOptions = PrintSavePreference.PrintSave.PrintOnly;
        }

        private void EP_DefaultFileFolder_TB_TextChanged(object sender, EventArgs e)
        {
            localPreferences.DefaultSaveDirectory = EP_DefaultFileFolder_TB.Text;
        }

        private void findDefaultFolderLocationExecute(object sender, EventArgs e)
        {
            using (var fbd = new FolderBrowserDialog())
            {
                DialogResult result = fbd.ShowDialog();

                if (result == DialogResult.OK && !string.IsNullOrWhiteSpace(fbd.SelectedPath))
                {
                    EP_DefaultFileFolder_TB.Text = fbd.SelectedPath;
                    localPreferences.DefaultSaveDirectory = fbd.SelectedPath;
                }
            }
        }

        private void EP_DefaultFileFolder_TB_Click(object sender, EventArgs e)
        {
            findDefaultFolderLocationExecute(sender, e);
        }

        private void EP_BrowseFolderLocation_BTN_Click(object sender, EventArgs e)
        {
            findDefaultFolderLocationExecute(sender, e);
        }

        private void EP_RentRosterLocation_TB_TextChanged(object sender, EventArgs e)
        {
            localPreferences.ExcelWorkBookFullFileSpec = EP_RentRosterLocation_TB.Text;
        }

        private void findTenantRosterExcelFile(object sender, EventArgs e)
        {
            string tenantRosterFile = "";
            OpenFileDialog FindTenantRoster = new OpenFileDialog();
            FindTenantRoster.InitialDirectory = "c:\\";
            FindTenantRoster.Filter = "Excel Files| *.xls; *.xlsx; *.xlsm";
            if (FindTenantRoster.ShowDialog() == DialogResult.OK)
            {
                tenantRosterFile = FindTenantRoster.FileName;
                EP_RentRosterLocation_TB.Text = tenantRosterFile;
                localPreferences.ExcelWorkBookFullFileSpec = tenantRosterFile;
            }
        }

        private void fillSheetSelectorListBox()
        {
            List<string> sheetNames = null;
            if (!string.IsNullOrEmpty(localPreferences.ExcelWorkBookFullFileSpec))
            {
                ExcelFileData ExcelFile = new ExcelFileData(localPreferences.ExcelWorkBookFullFileSpec,
                    localPreferences.ExcelWorkSheetName);
                sheetNames = ExcelFile.GetWorkSheetCollection();
            }

            if (sheetNames == null)
            {
                return;
            }

            EP_RentRosterSheetName_LISTBOX.DataSource = sheetNames;
            EP_RentRosterSheetName_LISTBOX.Visible = true;
            EP_SheetName_TB.Enabled = true;
        }
        private void EP_FindRenterRoster_BTN_Click(object sender, EventArgs e)
        {
            findTenantRosterExcelFile(sender, e);
            fillSheetSelectorListBox();
        }

        private void EP_RentRosterLocation_TB_Click(object sender, EventArgs e)
        {
            findTenantRosterExcelFile(sender, e);
            fillSheetSelectorListBox();
        }
        private void EP_RentRosterSheetName_LISTBOX_SelectedIndexChanged(object sender, EventArgs e)
        {
            localPreferences.ExcelWorkSheetName = EP_RentRosterSheetName_LISTBOX.SelectedItem.ToString();
            EP_SheetName_TB.Text = localPreferences.ExcelWorkSheetName;
        }

        private void EP_SheetName_TB_Click(object sender, EventArgs e)
        {
            fillSheetSelectorListBox();
        }

        private void EP_SavePreferences_BTN_Click(object sender, EventArgs e)
        {
            // Make sure any previous edits to the tenant are saved
            // then reset all the models.
            Globals.Save();
            Globals.ReInitizeAllModels(localPreferences);
            Globals.SavePreferences();
            DialogResult = DialogResult.OK;
            Close();
        }

        private void EP_Cancel_Button_Click(object sender, EventArgs e)
        {
            DialogResult = DialogResult.Cancel;
            Close();
        }
    }
}

PrintMailboxListsDlg.cs

This form allows the user to print and or save word documents created from the excel data. It also allows the user to override some of their preferences on a case by case basis.

using System;
using System.Collections.Generic;
using System.Windows.Forms;

namespace TenantRosterAutomation
{
    public partial class PrintMailboxListsDlg : Form
    {
        private PrintSavePreference.PrintSave printSave;
        private bool addDateToFileName = false;
        private bool addDateToTitle = false;
        private string selectedBuildings;
        private MSWordInterface wordInteropMethods;

        public PrintMailboxListsDlg()
        {
            InitializeComponent();
            wordInteropMethods = new MSWordInterface(Globals.Preferences);
        }

        private void PrintMailboxLists_Form_Load(object sender, EventArgs e)
        {
            List<string> buildings = Globals.Complex.BuildingAddressList;

            foreach (string building in buildings)
            {
                SelectBuilding2Print_listBox.Items.Add(building);
            }
            SelectBuilding2Print_listBox.Items.Add("All Buildings");

            if (Globals.Preferences.HavePreferenceData)
            {
                printSave = Globals.Preferences.PrintSaveOptions;
                PrintSaveChange();
            }

            AddDateToFileName_CB.Checked = addDateToFileName;
            AddDateUnderAddress_CB.Checked = addDateToTitle;
            PML_SaveAndPrint_Button.Enabled = false;
        }

        private void AddDateToFileName_CB_CheckedChanged(object sender, EventArgs e)
        {
            addDateToFileName = !addDateToFileName;
        }

        private void AddDateUnderAddress_CB_CheckedChanged(object sender, EventArgs e)
        {
            addDateToTitle = !addDateToTitle;
        }

        private void PML_SaveAndPrint_Button_Click(object sender, EventArgs e)
        {
            if (String.Compare(selectedBuildings, "All Buildings") == 0)
            {
                List<int> StreetNumbers = Globals.Complex.StreetNumbers;

                foreach (int streetNumber in StreetNumbers)
                {
                    printAndOrSaveMailList(streetNumber);
                }
            }
            else
            {
                string streetAddress = selectedBuildings.Substring(0, 5);
                printAndOrSaveMailList(streetAddress);
            }

            Close();
        }

        private void printAndOrSaveMailList(string streetAddress)
        {
            int iStreetNumber = 0;
            if (Int32.TryParse(streetAddress, out iStreetNumber))
            {
                printAndOrSaveMailList(iStreetNumber);
            }
            else
            {
                MessageBox.Show("Non Numeric string passed into PrintMailboxLists_Form::printAndOrSaveMailList().");
            }
        }

        private void PML_PrintOnly_RB_CheckedChanged(object sender, EventArgs e)
        {
            printSave = PrintSavePreference.PrintSave.PrintOnly;
        }

        private void PML_SavelOnly_RB_CheckedChanged(object sender, EventArgs e)
        {
            printSave = PrintSavePreference.PrintSave.SaveOnly;
        }

        private void PML_PrintAndSave_RB_CheckedChanged(object sender, EventArgs e)
        {
            printSave = PrintSavePreference.PrintSave.PrintAndSave;
        }

        private void PrintSaveChange()
        {
            switch (printSave)
            {
                case PrintSavePreference.PrintSave.PrintAndSave:
                    PML_PrintAndSave_RB.Checked = true;
                    break;

                case PrintSavePreference.PrintSave.SaveOnly:
                    PML_SavelOnly_RB.Checked = true;
                    break;

                default:
                    PML_PrintOnly_RB.Checked = true;
                    break;
            }
        }

        private void SelectBuilding2Print_listBox_SelectedIndexChanged(object sender, EventArgs e)
        {
            if (SelectBuilding2Print_listBox.SelectedItem != null)
            {
                selectedBuildings = SelectBuilding2Print_listBox.SelectedItem.ToString();
                PML_SaveAndPrint_Button.Enabled = true;
            }
        }

        private void printAndOrSaveMailList(int streetAddress)
        {
            bool save = ((printSave == PrintSavePreference.PrintSave.PrintAndSave) ? true :
                (printSave == PrintSavePreference.PrintSave.SaveOnly) ? true : false);
            bool print = ((printSave == PrintSavePreference.PrintSave.PrintAndSave) ? true :
                (printSave == PrintSavePreference.PrintSave.PrintOnly) ? true : false);

            string documentName = "MailboxList_" + streetAddress;

            string statusMessage = (print && save) ? "Printing and Saving " :
                (print) ? "Printing " : "Saving ";
            statusMessage += "the mailbox list for " + streetAddress;

            ReportCurrentStatusWindow psStatus = new ReportCurrentStatusWindow();
            psStatus.MessageText = statusMessage;
            psStatus.Show();

            Building building = Globals.Complex.GetBuilding(streetAddress);
            if (building != null)
            {
                MailboxData mailboxList = Globals.Complex.GetMailBoxList(building);
                if (mailboxList != null)
                {
                    wordInteropMethods.CreateMailistPrintAndOrSave(documentName,
                        mailboxList, addDateToFileName, addDateToTitle, save, print);
                }
            }

            psStatus.Close();
        }

    }
}

ApartmentNumberVerifier.cs

This form presents only a textbox and a button. It is called prior to calling either the Add or Edit Tenant Dialog or the Delete Tenant Dialog. The purpose of the dialog is to find the correct tenant by apartment number.

using System;
using System.Drawing;
using System.Windows.Forms;

namespace TenantRosterAutomation
{
    public partial class ApartmentNumberVerifier : Form
    {
        public enum NextActionEnum
        {
            ADD,
            DELETE,
            EDIT
        }

        public int ApartmentNumber;
        public Tenant TenantData { get; private set; }
        public NextActionEnum NextAction { get; set; }

        public ApartmentNumberVerifier()
        {
            TenantData = null;
            InitializeComponent();
        }

        private void APV_FindApartment_BTN_Click(object sender, EventArgs e)
        {
            VerifyApartmentNumber();
        }

        private void ANV_ApartmentNumber_TB_TextChanged(object sender, EventArgs e)
        {
            ANV_FindApartment_BTN.Enabled = true;
        }

        private void ApartmentNumberVerifier_Form_Load(object sender, EventArgs e)
        {
            ANV_FindApartment_BTN.Enabled = false;
            ANV_ApartmentNumber_TB.KeyDown += new KeyEventHandler(ANV_ApartmentNumber_TB_KeyDown);
        }

        private void ANV_ApartmentNumber_TB_KeyDown(object sender, KeyEventArgs e)
        {
            if (e.KeyCode == Keys.Enter)
            {
                VerifyApartmentNumber();
            }
        }

        private void ErrorActions(string errorMessage)
        {
            ANV_ApartmentNumber_TB.BackColor = Color.Yellow;
            ANV_ApartmentNumber_TB.Text = "";
            MessageBox.Show(errorMessage);
            ActiveControl = ANV_ApartmentNumber_TB;
        }

        private bool ReportErrors(int aptNumber, PropertyComplex.ApartmentNumberValid validApartmentId)
        {
            bool hasErrors = false;
            string errorMessage = null;

            switch (validApartmentId)
            {
                case PropertyComplex.ApartmentNumberValid.APARTMENT_NUMBER_NONNUMERIC:
                    errorMessage = "Please enter a number in the box.";
                    break;

                case PropertyComplex.ApartmentNumberValid.APARTMENT_NUMBER_OUT_OF_RANGE:
                    int minAptNo = Globals.Complex.MinApartmentNumber;
                    int maxAptNo = Globals.Complex.MaxApartmentNumber;
                    errorMessage = "The apartment number: " + aptNumber.ToString() +
                        " is out of range[" + minAptNo.ToString() +
                        ", " + maxAptNo.ToString() + "] please enter a valid apartment number.";
                    break;

                case PropertyComplex.ApartmentNumberValid.APARTMENT_NUMBER_NOT_FOUND:
                    errorMessage = "The number entered: " + aptNumber +
                        " was not found in the list of apartments.";
                    break;

                default:
                    break;
            }

            if (!string.IsNullOrEmpty(errorMessage))
            {
                ErrorActions(errorMessage);
                hasErrors = true;
            }

            return hasErrors;
        }

        private void VerifyApartmentNumber()
        {
            int aptNumber = 0;
            PropertyComplex.ApartmentNumberValid validApartmentId =
                Globals.Complex.VerifyApartmentNumber(ANV_ApartmentNumber_TB.Text, out aptNumber);

            if (ReportErrors(aptNumber, validApartmentId))
            {
                return;
            }

            ExecuteNextgAction(aptNumber);
            Close();
        }

        private void ExecuteNextgAction(int aptNumber)
        {
            TenantData = Globals.TenantRoster.GetTenant(aptNumber);

            switch (NextAction)
            {
                case NextActionEnum.EDIT:
                case NextActionEnum.ADD:
                    AddOrEditResidentDlg addNewResident = new AddOrEditResidentDlg();
                    addNewResident.CurrentTenant = TenantData;
                    addNewResident.ApartmentNumber = aptNumber;
                    addNewResident.Show();
                    break;

                case NextActionEnum.DELETE:
                    DeleteRenterDlg deleteRenter_dlg = new DeleteRenterDlg();
                    deleteRenter_dlg.CurrentTenant = TenantData;
                    deleteRenter_dlg.ApartmentNumber = aptNumber;
                    deleteRenter_dlg.Show();
                    break;

                default:
                    MessageBox.Show("Programmer error: NextAction = " +
                        NextAction.ToString() +
                        " this action is not implemented.");
                    break;
            }
        }
    }
}

AddOrEditResidentDlg.cs

This form allows the data in the excel file to be changed by either adding or editing the tenant information. This changes are saved in the TenantDataTable until the user either clicks the Save button or the Quit button. Both actions will save the data to the Excel spreadsheet.

using System;
using System.Drawing;
using System.Windows.Forms;

namespace TenantRosterAutomation
{
    public partial class AddOrEditResidentDlg : Form
    {
        public int ApartmentNumber { get; set; }
        public Tenant CurrentTenant { get; set; }

        public AddOrEditResidentDlg()
        {
            InitializeComponent();
        }

        private void ANR_SaveNewTenant_BTN_Click(object sender, EventArgs e)
        {
            CurrentTenant.LastName = ANR_TenantLastName_TB.Text;
            CurrentTenant.FirstName = ANR_TenantFirstName_TB.Text;
            CurrentTenant.LeaseStart = ANR_MoveInDate_TB.Text;
            CurrentTenant.LeaseEnd = ANR_LeaseEnd_TB.Text;
            CurrentTenant.HomePhone = ANR_HomePhone_TB.Text;
            CurrentTenant.CoTenantLastName = ANR_CoTenantLastName_TB.Text;
            CurrentTenant.CoTenantFirstName = ANR_AdditionalOccupantFirstName_TB.Text;
            CurrentTenant.RentersInsurancePolicy = ANR_RenterInsurance_TB.Text;
            CurrentTenant.Email = ANR_AlternateContact_TB.Text;
            Globals.TenantRoster.AddEditTenant(ApartmentNumber, CurrentTenant);
            Close();
        }

        private void AddNewResident_Form_Load(object sender, EventArgs e)
        {
            SetUpApartmentAddressLabel();
            ANR_SaveNewTenant_BTN.BackColor = Color.Green;
            ANR_Cancel_BTN.BackColor = Color.Red;
            if (CurrentTenant != null)
            {
                ANR_TenantLastName_TB.Text = CurrentTenant.LastName;
                ANR_TenantFirstName_TB.Text = CurrentTenant.FirstName;
                ANR_HomePhone_TB.Text = CurrentTenant.HomePhone;
                ANR_MoveInDate_TB.Text = CurrentTenant.LeaseStart;
                ANR_LeaseEnd_TB.Text = CurrentTenant.LeaseEnd;
                ANR_CoTenantLastName_TB.Text = CurrentTenant.CoTenantLastName;
                ANR_AdditionalOccupantFirstName_TB.Text = CurrentTenant.CoTenantFirstName;
                ANR_AlternateContact_TB.Text = CurrentTenant.Email;
                ANR_RenterInsurance_TB.Text = CurrentTenant.RentersInsurancePolicy;
            }
        }

        private void SetUpApartmentAddressLabel()
        {
            string apartmentFullAddress = ApartmentNumber.ToString();
            string buildingAddress =
                Globals.Complex.FindBuildingByApartment(ApartmentNumber);
            if (!string.IsNullOrEmpty(buildingAddress))
            {
                apartmentFullAddress = buildingAddress + " Apartment " + ApartmentNumber.ToString();
                ANR_ApartmentNumber_Label.Text = apartmentFullAddress;
            }
            else
            {
                ANR_ApartmentNumber_Label.Text = "Apartment Number:      " + apartmentFullAddress;
            }
            ANR_ApartmentNumber_Label.Font = new Font("Arial", 12, FontStyle.Bold);
        }

        private void ANR_Cancel_BTN_Click(object sender, EventArgs e)
        {
            Close();
        }
    }
}

DeleteRenterDlg.cs

This form allow sht user to delete a tenant from the apartment complex. The changes are also stored in the TenantDataTable.

using System;
using System.Drawing;
using System.Windows.Forms;

namespace TenantRosterAutomation
{
    public partial class DeleteRenterDlg : Form
    {
        public int ApartmentNumber { get; set; }
        public Tenant CurrentTenant { get; set; }

        public DeleteRenterDlg()
        {
            ApartmentNumber = 0;
            CurrentTenant = null;
            InitializeComponent();
        }

        private void DR_DeleteRenter_BTN_Click(object sender, EventArgs e)
        {
            Globals.TenantRoster.DeleteTenant(ApartmentNumber);
            Close();
        }

        private void DeleteRenter_Load(object sender, EventArgs e)
        {
            if (ApartmentNumber != 0 && CurrentTenant != null)
            {
                SetUpApartmentAddressLabel();
                DR_DeleteRenter_BTN.Enabled = false;
                DR_DeleteRenter_BTN.BackColor = Color.Red;
                DR_Cancel_BTN.BackColor = Color.Green;
                DR_TenantName_TB.Enabled = false;

                DR_TenantName_TB.Text = CurrentTenant.mergedName();
            }
        }

        private void SetUpApartmentAddressLabel()
        {
            string apartmentFullAddress = ApartmentNumber.ToString();
            string buildingAddress =
                Globals.Complex.FindBuildingByApartment(ApartmentNumber);
            if (!string.IsNullOrEmpty(buildingAddress))
            {
                apartmentFullAddress = buildingAddress + " Apartment " + ApartmentNumber.ToString();
                DR_AptNumber_LAB.Text = apartmentFullAddress;
            }
            else
            {
                DR_AptNumber_LAB.Text = "Apartment Number:      " + apartmentFullAddress;
            }
            DR_AptNumber_LAB.Font = new Font("Arial", 12, FontStyle.Bold);
        }



        private void DR_Renter2DeleteYes_RB_CheckedChanged(object sender, EventArgs e)
        {
            DR_DeleteRenter_BTN.Enabled = true;
            DR_DeleteRenter_BTN.BackColor = Color.Green;
            DR_Cancel_BTN.BackColor = Color.Red;
        }

        private void DR_Renter2DeleteNo_RB_CheckedChanged(object sender, EventArgs e)
        {
            DR_DeleteRenter_BTN.Enabled = false;
            DR_DeleteRenter_BTN.BackColor = Color.Red;
            DR_Cancel_BTN.BackColor = Color.Green;
        }

        private void DR_Cancel_BTN_Click(object sender, EventArgs e)
        {
            Close();
        }
    }
}

ReportCurrentStatusWindow.cs

This form is used to present information to the user about the current status of the program it is only used during operations that can take noticible time such as reading the excel data file or writing the updates to the excel data file.

using System;
using System.Drawing;
using System.Windows.Forms;

namespace TenantRosterAutomation
{
    public partial class ReportCurrentStatusWindow : Form
    {
        public string MessageText { get; set; }
        public ReportCurrentStatusWindow()
        {
            InitializeComponent();
        }

        private void Form_CurrentProgressStatus_Load(object sender, EventArgs e)
        {
            CPS_Message_TB.Text = MessageText;
            CPS_Message_TB.Font = new Font("Arial", 12, FontStyle.Bold);
            Size size = TextRenderer.MeasureText(CPS_Message_TB.Text, CPS_Message_TB.Font);
            Height = (size.Height * 3) + 84;
            Width = size.Width + 84;
            CPS_Message_TB.Width = size.Width;
            CPS_Message_TB.Height = size.Height * 3;
            CPS_Message_TB.Left = 40;
            CPS_Message_TB.Height = 40;
        }
    }
}
\$\endgroup\$
3
  • \$\begingroup\$ Consider MVP pattern. Use data bindings? INotifyPropertyChanged? Also take care about field/property names: Snake_Case is not a thing to use in C#. Avoid absolute pixel-sizing or pixel-positioning, use panels like FlowLayoutPanel/TableLayoutPanel to arrange the UI components instead. \$\endgroup\$
    – aepot
    Commented Jan 17, 2022 at 2:22
  • \$\begingroup\$ @aepot Write it up as an answer and get +1 and probably accepted. \$\endgroup\$
    – pacmaninbw
    Commented Jan 17, 2022 at 2:40
  • \$\begingroup\$ It doesn't look like review but a couple of tips. I'm not ready to post the whole review, sorry. You may also create a self-answer then. \$\endgroup\$
    – aepot
    Commented Jan 17, 2022 at 3:10

2 Answers 2

3
\$\begingroup\$

RentRosterApp

  • If you have read any of my reviews then you know I do value consistent naming
    • Here I can see occasional abbreviations which does not help legibility
      • ..._Button_Click vs ..._BTN_Click
      • ApartmentNumberVerifier vs EditPreferencesDlg
      • EditPreferences_... vs RR_Quit_...
  • The Load method can be greatly simplified
private void RentRosterApp_Load(object sender, EventArgs e)
{
    RR_Quit_BTN.BackColor = Color.Red;
    if (globalsInitialized) return; //early exit

    SetButtonsEnablement(false);

    using (EditPreferencesDlg preferences_dlg = new EditPreferencesDlg())
        if (preferences_dlg.ShowDialog() == DialogResult.OK)
            SetButtonsEnablement(true);
}

private void SetButtonsEnablement(bool isEnabled)
{
    PrintMailboxLists_Button.Enabled = isEnabled;
    AddNewResident_Button.Enabled = isEnabled;
    DeleteRenter_Button.Enabled = isEnabled;
}
  • Inside the Load method you have used using around EditPreferencesDlg whereas in EditPreferences_BTN_Click you haven't
  • You can extract the common part for the RR button event handlers into a separate method
private void RR_Quit_BTN_Click(object sender, EventArgs e)
{
    SafeExcelCall(() =>
    {
        Globals.Save();
        Globals.ReleaseAllModels();
        Close();
    });
}

private void RR_SAVEEDITS_BTN_Click(object sender, EventArgs e)
{
    SafeExcelCall(Globals.SaveTenantData);
}

//Since I don't know your domain I can't find a really good name here
//Usually we prefix a method with `SafeXYZ` if it handles `XYZ` exceptions 
private void SafeExcelCall(Action excelCall)
{
    try
    {
        excelCall();
    }
    catch (AlreadyOpenInExcelException ao)
    {
        MessageBox.Show(ao.Message);
    }
}

EditPreferencesDlg

  • I don't see the need why do we need using block around this form
    • Please note that I haven't used WinForms in the last 10 years so I might be wrong on using
  • The Load method's switch statement can be rewritten like this to avoid repetitive code
RadioButton shouldBeCheckedRB;
switch (localPreferences.PrintSaveOptions)
{
    case PrintSavePreference.PrintSave.PrintAndSave:
        shouldBeCheckedRB = EP_PrintAndSave_RB;
        break;
    case PrintSavePreference.PrintSave.SaveOnly:
        shouldBeCheckedRB = EP_SavelOnly_RB;
        break;
    default:
        shouldBeCheckedRB = EP_PrintOnly_RB;
        break;
}
shouldBeCheckedRB.Checked = true;
  • Or if you can use switch expression (depending on your C# version) then you can end up with an even more concise code
RadioButton shouldBeCheckedRB = localPreferences.PrintSaveOptions switch
{
    PrintSavePreference.PrintSave.PrintAndSave => EP_PrintAndSave_RB,
    PrintSavePreference.PrintSave.SaveOnly => EP_SavelOnly_RB,
    _ => EP_PrintOnly_RB,
};
shouldBeCheckedRB.Checked = true;
  • Yet again if you want to reduce the repetitive code of your XYZ_CheckedChanged handlers then you can do the following
private void EP_PrintAndSave_RB_CheckedChanged(object sender, EventArgs e)
    => SetPrintSaveOptions(PrintSavePreference.PrintSave.PrintAndSave);

private void EP_SavelOnly_RB_CheckedChanged(object sender, EventArgs e)
    => SetPrintSaveOptions(PrintSavePreference.PrintSave.SaveOnly);  

private void EP_PrintOnly_RB_CheckedChanged(object sender, EventArgs e)
    => SetPrintSaveOptions(PrintSavePreference.PrintSave.PrintOnly);

private void SetPrintSaveOptions(PrintSavePreference.PrintSave option)
    => localPreferences.PrintSaveOptions = option;
  • Or you define a mapping between the RadioButtons and the PrintSave values and then dynamically specify the event handlers (for example inside Load)
Dictionary<RadioButtion, PrintSavePreference.PrintSave> mappings = new Dictionary<RadioButtion, PrintSavePreference.PrintSave>
{
    { EP_PrintAndSave_RB, PrintSavePreference.PrintSave.PrintAndSave },
    { EP_SavelOnly_RB, PrintSavePreference.PrintSave.SaveOnly },
    { EP_PrintOnly_RB, PrintSavePreference.PrintSave.PrintOnly }
};

foreach (var mapping in mappings)
{
    mapping.Key.CheckedChanged += (s, e) =>
        localPreferences.PrintSaveOptions = mapping.Value;
}
  • findDefaultFolderLocationExecute: this does not follow your snake casing naming
  • Once again the naming of your variables are inconsistent
    • findDefaultFolderLocationExecute's fbd vs findTenantRosterExcelFile's FindTenantRoster
  • Minor but you can use object initializer to reduce the initialization code
var FindTenantRoster = new OpenFileDialog()
{ 
    InitialDirectory = "c:\\",
    Filter = "Excel Files| *.xls; *.xlsx; *.xlsm"
};
  • And here comes my favourite control name: EP_RentRosterSheetName_LISTBOX
  • Please try to consolidate your variables names as well (ExcelFile vs sheetNames)
  • You can reduce the level of indention of the fillSheetSelectorListBox method by using early exits
if (string.IsNullOrEmpty(localPreferences.ExcelWorkBookFullFileSpec))
    return; 
            
var excelFile = new ExcelFileData(
    localPreferences.ExcelWorkBookFullFileSpec,
    localPreferences.ExcelWorkSheetName);
List<string> sheetNames = excelFile.GetWorkSheetCollection();
            
if (sheetNames == null)
    return;

PrintMailboxListsDlg

  • Inside the Load please prefer AddRange over multiple Add method calls
SelectBuilding2Print_listBox.Items.AddRange(Globals.Complex.BuildingAddressList);
SelectBuilding2Print_listBox.Items.Add("All Buildings");
  • If you move the CheckedChanged handlers inside the Load method then those piece of code that are related to each other (high cohesion) then they are next to each other
AddDateToFileName_CB.Checked = addDateToFileName;
AddDateToFileName_CB.CheckedChanged += (s, e) => addDateToFileName = !addDateToFileName;

AddDateUnderAddress_CB.Checked = addDateToTitle;
AddDateUnderAddress_CB.CheckedChanged += (s, e) => addDateToTitle = !addDateToTitle;
  • Inside the PML_SaveAndPrint_Button_Click you are using the same hard-coded string ("All Buildings") as you have used in the Load
    • Please prefer class level constants
  • String.Compare(...) == 0 could be replaced with string.Equals
string.Equals(selectedBuildings, allBuildings)
  • The printAndOrSaveMailList could be rewritten in a way that the core functionality is on the top indentation level
private void PrintAndOrSaveMailList(string streetAddress)
{
    if (!int.TryParse(streetAddress, out int streetNumber))
    {
        MessageBox.Show("Non Numeric string passed into PrintMailboxLists_Form::printAndOrSaveMailList().");
        return;
    }

    PrintAndOrSaveMailList(streetNumber);
}
  • BTW this iStreetNumber does sound like an apple product :)
    • Since the number suffix already indicates the data type I would not recommend to prefix it with i
  • Inside your SelectBuilding2Print_listBox_SelectedIndexChanged method I don't think that null check is needed
    • At least based on my intuition a listbox will not fire an selected index changed event for a null item
  • Inside your PrintAndOrSaveMailList I think you have overcomplicated the two bool variables' assignment
bool save = printSave == PrintSavePreference.PrintSave.PrintAndSave
    || printSave == PrintSavePreference.PrintSave.SaveOnly;
bool print = printSave == PrintSavePreference.PrintSave.PrintAndSave
    || printSave == PrintSavePreference.PrintSave.PrintOnly;
  • Or with Linq you can write something like this
bool save = new[] { PrintSavePreference.PrintSave.PrintAndSave, PrintSavePreference.PrintSave.SaveOnly }.Contains(printSave);
bool print = new[] { PrintSavePreference.PrintSave.PrintAndSave, PrintSavePreference.PrintSave.PrintOnly }.Contains(printSave);
  • For string concatenations please prefer string interpolation or StringBuilder instead of + or += operators
  • If you want to you get rid of one level of indentation at the end of this method then you can change the mailboxList assignment like this
Building building = Globals.Complex.GetBuilding(streetAddress);
MailboxData mailboxList = building != null ? Globals.Complex.GetMailBoxList(building) : null;

if (mailboxList != null)
{
    wordInteropMethods.CreateMailistPrintAndOrSave(documentName,
        mailboxList, addDateToFileName, addDateToTitle, save, print);
}

ApartmentNumberVerifier

public enum NextAction
{
    Add,
    Delete,
    Edit
}
  • It is perfectly normal to have properties like this
public NextAction NextAction { get; set; }
  • Inside your ANV_ApartmentNumber_TB_KeyDown please prefer early exit over guard expression to streamline the method's implementation
private void ANV_ApartmentNumber_TB_KeyDown(object sender, KeyEventArgs e)
{
    if (e.KeyCode != Keys.Enter)
        return;
            
    VerifyApartmentNumber();
}
  • The ReportError can be written in so many different ways, let me show two
  • A) With switch statement + hasError re-assignment in default case
bool hasError = true;
string errorMessage = null;
switch (validApartmentId)
{
    case PropertyComplex.ApartmentNumberValid.APARTMENT_NUMBER_NONNUMERIC:
        errorMessage = "Please enter a number in the box.";
        break;
    case PropertyComplex.ApartmentNumberValid.APARTMENT_NUMBER_OUT_OF_RANGE:
        errorMessage = $"The apartment number: {aptNumber} is out of range[{Globals.Complex.MinApartmentNumber}, {Globals.Complex.MaxApartmentNumber}] please enter a valid apartment number.";
        break;
    case PropertyComplex.ApartmentNumberValid.APARTMENT_NUMBER_NOT_FOUND:
        errorMessage = $"The number entered: {aptNumber} was not found in the list of apartments.";
        break;
    default:
        hasError = false;
        break;
}

if (hasError)
    ErrorActions(errorMessage);

return hasError;
  • B) With switch expression + hasError calculation after switch
string errorMessage = validApartmentId switch
{
    PropertyComplex.ApartmentNumberValid.APARTMENT_NUMBER_NONNUMERIC =>
        "Please enter a number in the box.",
    PropertyComplex.ApartmentNumberValid.APARTMENT_NUMBER_OUT_OF_RANGE =>
        $"The apartment number: {aptNumber} is out of range[{Globals.Complex.MinApartmentNumber}, {Globals.Complex.MaxApartmentNumber}] please enter a valid apartment number.",
    PropertyComplex.ApartmentNumberValid.APARTMENT_NUMBER_NOT_FOUND =>
        $"The number entered: {aptNumber} was not found in the list of apartments.",
    _ => null
};
       
var hasError = !string.IsNullOrEmpty(errorMessage);
if (hasError)
    ErrorActions(errorMessage);

return hasError;
  • ExecuteNextgAction: I assume the g is just a typo here
  • If you want to execute more than a single line in each case branch (+ break) then prefer to use code block {} statement
case NextActionEnum.Add:
    {
        AddOrEditResidentDlg addNewResident = new AddOrEditResidentDlg();
        addNewResident.CurrentTenant = TenantData;
        addNewResident.ApartmentNumber = aptNumber;
        addNewResident.Show();
        break;
    }
  • Or try to convert your multiline block into a single statement if possible
case NextAction.Add:
    new AddOrEditResidentDlg()
    {
        CurrentTenant = TenantData,
        ApartmentNumber = aptNumber
    }.Show();
    break;

AddOrEditResidentDlg

  • As you have already seen I'm not a huge fan of a repetitive code
    • When I saw here the manual DataBinding between the TextBoxes and the Tenant object I though I can come up with an easier solution
    • I did, but it relies on building expression trees dynamically which needs a bit more explanation than simply just sharing it with you
    • So, I decided to make another post with that code
  • The SetUpApartmentAddressLabel method can be greatly simplified, where there is no need for the if-else blocks and for the apartmentFullAddress variable
string buildingAddress =
    Globals.Complex.FindBuildingByApartment(ApartmentNumber);

ANR_ApartmentNumber_Label.Text = string.IsNullOrEmpty(buildingAddress)
    ? $"Apartment Number:      {ApartmentNumber}"
    : $"{buildingAddress} Apartment {ApartmentNumber}";

ANR_ApartmentNumber_Label.Font = new Font("Arial", 12, FontStyle.Bold);

DeleteRenterDlg

  • Inside the ctor you don't need to initialize the CurrentTenant with null, since that's the default value for reference types
    • Same applies for ApartmentNumber where the default value is 0
  • By the way it seems a bit unsafe that you do not perform any checks on the ApartmentNumber property before you call the DeleteTenant
  • Inside the Load method you can use the default operator/literal to check the properties' values
//prior C# 7.1
if (ApartmentNumber != default(int) && CurrentTenant != default(Tenant))
//since C# 7.1 we can use default literal
if (ApartmentNumber != default && CurrentTenant != default)
  • Yet again please prefer early exit instead of guard expression
if (ApartmentNumber == default || CurrentTenant == default)
    return;
            
SetUpApartmentAddressLabel();
...
  • The SetUpApartmentAddressLabel can be simplified in the same way as I suggested it for the AddOrEditResidentDlg class
  • You can combine the DR_Renter2DeleteYes_RB_CheckedChanged and DR_Renter2DeleteNo_RB_CheckedChanged event handlers into one:
private void DR_Renter2Delete_RB_CheckedChanged(object sender, EventArgs e)
{
    var rb = (RadioButton)sender;
    var canBeDeleted = string.Equals(rb.Name, nameof(DR_Renter2DeleteYes_RB));

    DR_DeleteRenter_BTN.Enabled = canBeDeleted;
    DR_DeleteRenter_BTN.BackColor = canBeDeleted ? Color.Green : Color.Red;
    DR_Cancel_BTN.BackColor = canBeDeleted ? Color.Red : Color.Green;
}

ReportCurrentStatusWindow

  • Either call the property Text or Message, you don't need to use both
    • It's like you name your warning label, which is very important to WarningVeryImporantInformationLabel
    • Either WarningLabel or ImportantInfoLabel would be sufficient
  • <sarcasm> I love magic numbers </sarcasm>
    • They can break your application really easily if you don't pay enough attention
    • A better way can be the utilization of constants
const int WindowMargin = 84;
const int MessageTBMargin = 40;
const int HeightMultipler = 3;

private void Form_CurrentProgressStatus_Load(object sender, EventArgs e)
{
    CPS_Message_TB.Text = MessageText;
    CPS_Message_TB.Font = new Font("Arial", 12, FontStyle.Bold);
    Size size = TextRenderer.MeasureText(CPS_Message_TB.Text, CPS_Message_TB.Font);

    Height = (size.Height * HeightMultipler) + WindowMargin;
    Width = size.Width + WindowMargin;
    CPS_Message_TB.Width = size.Width;
    CPS_Message_TB.Height = size.Height * HeightMultipler;
    CPS_Message_TB.Left = MessageTBMargin;
    CPS_Message_TB.Height = MessageTBMargin; //Did you mean Top?
}
  • By the way you are setting the CPS_Message_TB.Height twice
    • I suppose you wanted to set the Top property instead
\$\endgroup\$
5
  • 1
    \$\begingroup\$ Thank you for your very thorough review, I look forward to any continuation. \$\endgroup\$
    – pacmaninbw
    Commented Jan 18, 2022 at 15:27
  • 1
    \$\begingroup\$ @pacmaninbw I will try to finish my review at the end of tomorrow. \$\endgroup\$ Commented Jan 18, 2022 at 15:31
  • 1
    \$\begingroup\$ Re: I don't see the need why do we need using block around this form You are correct, there is no reason for a using block here although there might be good reason for a try {} catch() {} block. \$\endgroup\$
    – pacmaninbw
    Commented Jan 18, 2022 at 16:34
  • 1
    \$\begingroup\$ Wow, 23 new issues in the issues list. I've closed one because I won't address it. If you want to see how seriously I take code reviews look at the list. Old C/C++ programming habits for the names of the enums and the lack of class constants for strings, I will fix those. It is amazing how many good practices you ignore when you are in a rush for a deadline. \$\endgroup\$
    – pacmaninbw
    Commented Jan 18, 2022 at 18:59
  • \$\begingroup\$ @pacmaninbw I can see your dedication, awesome. I've updated my post please check the new observations as well. \$\endgroup\$ Commented Jan 19, 2022 at 8:11
2
\$\begingroup\$

I've already left a fairly lengthy post where I have detailed most of my observations.

Here I want to focus only on the AddOrEditResidentDlg class's DataBinding thingy.

As I said I'm not a huge fan of the repetitive code like these:

CurrentTenant.LastName = ANR_TenantLastName_TB.Text;
CurrentTenant.FirstName = ANR_TenantFirstName_TB.Text;
CurrentTenant.LeaseStart = ANR_MoveInDate_TB.Text;
CurrentTenant.LeaseEnd = ANR_LeaseEnd_TB.Text;
CurrentTenant.HomePhone = ANR_HomePhone_TB.Text;
CurrentTenant.CoTenantLastName = ANR_CoTenantLastName_TB.Text;
CurrentTenant.CoTenantFirstName = ANR_AdditionalOccupantFirstName_TB.Text;
CurrentTenant.RentersInsurancePolicy = ANR_RenterInsurance_TB.Text;
CurrentTenant.Email = ANR_AlternateContact_TB.Text;
ANR_TenantLastName_TB.Text = CurrentTenant.LastName;
ANR_TenantFirstName_TB.Text = CurrentTenant.FirstName;
ANR_HomePhone_TB.Text = CurrentTenant.HomePhone;
ANR_MoveInDate_TB.Text = CurrentTenant.LeaseStart;
ANR_LeaseEnd_TB.Text = CurrentTenant.LeaseEnd;
ANR_CoTenantLastName_TB.Text = CurrentTenant.CoTenantLastName;
ANR_AdditionalOccupantFirstName_TB.Text = CurrentTenant.CoTenantFirstName;
ANR_AlternateContact_TB.Text = CurrentTenant.Email;
ANR_RenterInsurance_TB.Text = CurrentTenant.RentersInsurancePolicy;

Basically it is a TwoWay binding between your CurrentTenant property and your TextBoxes. I don't want to convince you here to use DataBinding rather I would like to show you a programatic way to do that.


So, the above code basically says which property of the Tenant object is connected to the which TextBox's Text. If we can define a mapping then we can use that information in both directions. Fortunately we can :) In order to specify a property selector all you need is a simple Func like this:

Func<Tenant, string>

It says if I receive a Tenant instance I will return a string. All of the following lambda expressions are satisfy this requirement:

ct => ct.LastName
ct => ct.FirstName
...
ct => ct.Email

Great, we can define the mapping for example via a Dictionary

static readonly Dictionary<Func<Tenant, string>, TextBox> mappings = new Dictionary<Func<Tenant, string>, TextBox>
{
    { ct => ct.LastName, ANR_TenantLastName_TB },
    { ct => ct.FirstName, ANR_TenantFirstName_TB },
    { ct => ct.LeaseStart, ANR_MoveInDate_TB },
    { ct => ct.LeaseEnd, ANR_LeaseEnd_TB },
    { ct => ct.HomePhone, ANR_HomePhone_TB },
    { ct => ct.CoTenantLastName, ANR_CoTenantLastName_TB },
    { ct => ct.CoTenantFirstName, ANR_AdditionalOccupantFirstName_TB },
    { ct => ct.RentersInsurancePolicy, ANR_RenterInsurance_TB },
    { ct => ct.Email, ANR_AlternateContact_TB }
};

We are almost good, but we need to create assignment expressions dynamically, so we need to use Expression<Func<Tenant, string>> instead of Func<Tenant, string>. The good news is that we only need to change the Dictionary's type but we don't have to touch the collection initializer.

static readonly Dictionary<Expression<Func<Tenant, string>>, TextBox> mappings = new Dictionary<Expression<Func<Tenant, string>>, TextBox>
{
    { ct => ct.LastName, ANR_TenantLastName_TB },
    { ct => ct.FirstName, ANR_TenantFirstName_TB },
    { ct => ct.LeaseStart, ANR_MoveInDate_TB },
    { ct => ct.LeaseEnd, ANR_LeaseEnd_TB },
    { ct => ct.HomePhone, ANR_HomePhone_TB },
    { ct => ct.CoTenantLastName, ANR_CoTenantLastName_TB },
    { ct => ct.CoTenantFirstName, ANR_AdditionalOccupantFirstName_TB },
    { ct => ct.RentersInsurancePolicy, ANR_RenterInsurance_TB },
    { ct => ct.Email, ANR_AlternateContact_TB }
};

Now let's see how can we make use of this.

Let's start with the update Tenant property with a TextBox's Text value

private void UpdateCurrentTenantWithTextBoxValue(Expression<Func<Tenant, string>> propertySelector, TextBox textBox)
{
    var stringParameter = Expression.Parameter(typeof(string));
    Expression<Func<TextBox, string>> textSelector = tb => tb.Text;

    var assignment = Expression.Lambda<Action<Tenant, string>>(
        Expression.Assign(propertySelector.Body, stringParameter),
        propertySelector.Parameters[0], stringParameter);

    var textBoxTextValue = textSelector.Compile()(textBox);
    assignment.Compile()(CurrentTenant, textBox.Text);
}
  • The two parameters of this method are nicely aligned with the Dictionary's Key and Value
  • First we create a ParameterExpression where we are stating that our dynamically created assignment expression would require a string parameter (the new value)
  • Then we define a property selector for a TextBox
    • Here we specify that we want to use the Text property of the given TextBox instance
  • Then we define our assignment expression
    • You should think about this as a method which receives two parameters (a Tenant and a string) and then performs the assignment
(Tenant t, string newValue) => propertySelector(t) = newValue;
  • I would not go into the details why this expression was built up like this (if you don't mind)
  • Finally get the actual value of the TextBox's Text by complying and executing the textSelector
    • So, the textBoxTextValue contains the actual value of the textBox parameter's Text
  • We need to compile and execute the assignment as well where we specify its parameters
    • So, basically the above assignment expression is called like this
assignment(CurrentTenant, textSelector(textBox))

With this tool in our hand we can simplify the ANR_SaveNewTenant_BTN_Click method like this

private void ANR_SaveNewTenant_BTN_Click(object sender, EventArgs e)
{
    foreach (var mapping in mappings)
        UpdateCurrentTenantWithTextBoxValue(mapping.Key, mapping.Value);

    Globals.TenantRoster.AddEditTenant(ApartmentNumber, CurrentTenant);
    Close();
}

The other direction can be also greatly simplified as well

private void AddNewResident_Form_Load(object sender, EventArgs e)
{
    SetUpApartmentAddressLabel();
    ANR_SaveNewTenant_BTN.BackColor = Color.Green;
    ANR_Cancel_BTN.BackColor = Color.Red;

    if (CurrentTenant == null)
        return;

    foreach (var mapping in mappings)
        UpdateTextBoxWithCurrentTenantValue(mapping.Value, mapping.Key);
}

Let's see the code of the UpdateTextBoxWithCurrentTenantValue

private static void UpdateTextBoxWithCurrentTenantValue(TextBox textBox, Expression<Func<Tenant, string>> propertySelector)
{
    var stringParameter = Expression.Parameter(typeof(string));
    Expression<Func<TextBox, string>> textSelector = tb => tb.Text;
    var assignment = Expression.Lambda<Action<TextBox, string>>(
        Expression.Assign(textSelector.Body, stringParameter),
        textSelector.Parameters[0], stringParameter);

    var tenantFieldValue = propertySelector.Compile()(CurrentTenant);
    assignment.Compile()(textBox, tenantFieldValue);
}
  • It is the symmetric counterpart of the previous method
  • Here I've change the order of the parameters because for me it makes more sense
    • But feel free to keep the parameter ordering as the other one if you wish
  • The only difference here is the assignment expression
(TextBox tb, string newValue) => textSelector(tb) = newValue;
  • The actual call of this expression look like this
assignment(textBox, propertySelector(CurrentTenant))

As you can see the ParameterExpression and the textSelector can be easily extracted from both methods

private static ParameterExpression StringParameter = Expression.Parameter(typeof(string));
private static Expression<Func<TextBox, string>> TextSelector = tb => tb.Text;

private void UpdateCurrentTenantWithTextBoxValue(Expression<Func<Tenant, string>> propertySelector, TextBox textBox)
{
    var assignment = Expression.Lambda<Action<Tenant, string>>(
        Expression.Assign(propertySelector.Body, StringParameter),
        propertySelector.Parameters[0], StringParameter);

    var textBoxTextValue = TextSelector.Compile()(textBox);
    assignment.Compile()(CurrentTenant, textBox.Text);
}

private static void UpdateTextBoxWithCurrentTenantValue(TextBox textBox, Expression<Func<Tenant, string>> propertySelector)
{
    var assignment = Expression.Lambda<Action<TextBox, string>>(
        Expression.Assign(TextSelector.Body, StringParameter),
        TextSelector.Parameters[0], StringParameter);

    var tenantFieldValue = propertySelector.Compile()(CurrentTenant);
    assignment.Compile()(textBox, tenantFieldValue);
}

I know this solution is more complex than a simple databinding and it requires a bit more advanced C# skills but it does work. It is an alternative feel free to give it a try.

\$\endgroup\$
3
  • 2
    \$\begingroup\$ @aepot nice catch, thanks mate \$\endgroup\$ Commented Jan 19, 2022 at 18:06
  • 1
    \$\begingroup\$ I may have to change the accepted answer, definitely interesting. \$\endgroup\$
    – pacmaninbw
    Commented Jan 19, 2022 at 19:10
  • \$\begingroup\$ @pacmaninbw I'm glad that you have found it interesting. :) \$\endgroup\$ Commented Jan 19, 2022 at 19:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.