Что не так с этим классом? (дизайн мудрый) - PullRequest
0 голосов
/ 26 декабря 2009

Исходя из этого вопроса: разработка классов приложений

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

Я пытаюсь реорганизовать этот класс и его абстрактный базовый класс (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
    }

Ответы [ 3 ]

4 голосов
/ 26 декабря 2009

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

2 голосов
/ 26 декабря 2009

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

0 голосов
/ 26 декабря 2009

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

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...