Что не так с этим классом? (Дизайн мудрый)

Question

Следуя из этого вопроса: designing application classes

Что не так (с точки зрения дизайна) с этим классом:

Я пытаюсь реорганизовать этот класс, и это абстрактный базовый класс (Logon) и тот факт, что это действительно ужасный дизайн. Я написал это сам (в основном, когда был новичком). Мне сложно реорганизовать и вам нужен какой-то вклад?

class NewUserLogon : Logon, ILogonNewUser, IDisposable 
    { 
     #region Member Variables 

     System.Windows.Forms.Form _frm = new MainWindow(); 

     SQLDatabase.SQLDynamicDatabase sql; 
     SQLDatabase.DatabaseLogin dblogin; 
     LogonData lgndata; 
     System.Security.SecureString securepassword; 
     PasswordEncrypt.Collections.CreatedItems items; 

     LogonEventArgs e = new LogonEventArgs(); 

     #endregion 

     #region Constructors 
     // for DI 
     public NewUserLogon(PasswordEncrypt.Collections.CreatedItems items) : base (items) 
     { 
      this.items = items; 
     } 
     #endregion 

     #region Public Methods 
     public new void Dispose() 
     { 
     } 

     public bool? ReadFromRegistry(HashedUsername username, HashedPassword hashedpassword) 
     { 
      return RegistryEdit.ReadFromRegistry(username, hashedpassword); 
     } 

     public bool WriteToRegistry(HashedUsername username, HashedPassword hashedpassword) 
     { 
      return RegistryEdit.WriteToRegistry(username, hashedpassword); 
     } 

     public override void Login(TextBox username, TextBox password) 
     { 
      base.Login(username, password); 
      Login(username.Text, password.Text); 
     } 
     #endregion 

     #region Protected Methods 
     protected override void Login(string username, string password) // IS INSECURE!!! ONLY USE HASHES 
     { 
      base.Login(username, password); 

      if (_user is NewUserLogon) // new user 
      { 
       sql = new PasswordEncrypt.SQLDatabase.SQLDynamicDatabase(); 
       dblogin = new SQLDatabase.DatabaseLogin(); 
       lgndata = base._logondata; 
       securepassword = new System.Security.SecureString(); 

       // Set Object for eventhandler 
       items.SetDatabaseLogin = dblogin; 
       items.SetSQLDynamicDatabase = sql; // recreates L 
       items.Items = items; 

       string generatedusername; 

       // write new logondata to registry 
       if (this.WriteToRegistry(lgndata.HahsedUserName, lgndata.HashedPsw)) 
       { 
        try 
        { 
         // Generate DB Password... 
         dblogin.GenerateDBPassword(); 

         // get generated password into securestring 
         securepassword = dblogin.Password; 

         //generate database username 
         generatedusername = dblogin.GenerateDBUserName(username); 

         if (generatedusername == "Already Exists") 
         { 
          throw new Exception("Username Already Exists"); 
         } 

         //create SQL Server database 
         try 
         { 
          sql.CreateSQLDatabase(dblogin, username); 
         } 
         catch (Exception ex) 
         { 
          //System.Windows.Forms.MessageBox.Show(ex.Message); 
          e.ErrorMessage = ex.Message; 
          e.Success = false; 
          OnError(this, e); 
         } 
        } 
        catch (Exception exc) 
        { 
         e.Success = false; 
         e.ErrorMessage = exc.Message; 
         OnError(this, e); 
        } 
        OnNewUserLoggedIn(this, e); // tell UI class to start loading... 
       } 
       else 
       { 
        System.Windows.Forms.MessageBox.Show("Unable to write to Registry!", "Registry Error", System.Windows.Forms.MessageBoxButtons.OK, System.Windows.Forms.MessageBoxIcon.Exclamation); 
       } 
      } 

      else if (_user is ExistingUserLogon) // exising user 
      { 

       bool? compare = base._regRead; 
       lgndata = base._logondata; 

       if (compare == true) 
       { 
        //Tell GUI to quit the 'busydialog' thread 
        OnMessage(this, e); 
        LogonFrm frm = LogonFrm.LogonFormInstance; 

        // tell user he already exists and just needs to login 
        // ask if user wants to logon straight away 
        System.Windows.Forms.DialogResult dlgres; 
        dlgres = System.Windows.Forms.MessageBox.Show("Your login already exists, do you wan to login now?", "Login Exists", System.Windows.Forms.MessageBoxButtons.YesNo, System.Windows.Forms.MessageBoxIcon.Question); 

        if (dlgres == System.Windows.Forms.DialogResult.Yes) 
        { 
         ExistingUserLogon existinguser = new ExistingUserLogon(compare, lgndata); 
         existinguser.Error += new ErrorStatus(frm._newuser_Error); 
         existinguser.loginname = username; 
         existinguser.LoginNewUser(); 

         ///TELL GUI THAT USER LOGIN SUCCEEDED, THROUGH EVENT 
         e.Success = true; 
         OnNewUserLoggedIn(this, e); 

        } 
        else 
        { 
         e.Success = false; 
         e.ErrorMessage = "Failed"; 
         OnError(this, e); 

        } 
       } 

      } 


     } 
     #endregion 
    } 

Answer 1

Ваш класс пытается сделать слишком много вещей. Попытайтесь разделить разные обязанности на отдельные классы (например, доступ к базе данных и материал пользовательского интерфейса).
И почему вы создаете новую форму в начале своего класса и, похоже, не используете ее дальше?

Answer 2

Ваш protected Login слишком длинный.

Answer 3

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