7
\$\begingroup\$

I have a working logon for a WinForms Project. I use the program.cs file to launch the login form. I am not so sure there isn't a better way of implementing this.

Here is my program.cs file:

    using System;
    using System.Windows.Forms;
    using WindowsFormsApp.Presenters;
    using WindowsFormsApp.Views;
    using Autofac;
    using DbContexts;
    using Serilog;

    namespace WindowsFormsApp
    {
        internal static class Program
        {
            public static IContainer Container { get; private set; }
            public static string UserName { get; set; }

            /// <summary>
            /// The main entry point for the application.
            /// </summary>
            [STAThread]
            private static void Main()
            {
                var builder = new ContainerBuilder();

                builder.Register(c => new MyContext());
            
                Container = builder.Build();

                Log.Logger = new LoggerConfiguration()
                    .MinimumLevel.Debug()
                    .WriteTo.Console()
                    .WriteTo.RollingFile("log-{Date}.txt")
                    .CreateLogger();
                Log.Information("Application Started");

                Application.EnableVisualStyles();
                Application.SetCompatibleTextRenderingDefault(false);

                var loginForm = new LoginForm();
                var results = loginForm.ShowDialog();

                if (results == DialogResult.Cancel)
                    System.Environment.Exit(1);

                while (results != DialogResult.OK)
                {
                    results = loginForm.ShowDialog();
                    if (results == DialogResult.Cancel)
                        System.Environment.Exit(1);
                }

                var mainFormView = new MainFormView();

                mainFormView.Tag = new MainFormPresenter(mainFormView);

                Application.Run(mainFormView);
            }
        }
    }

Any suggestions or comments are welcome.

\$\endgroup\$
4
\$\begingroup\$
  1. You have a public static property UserName that can be set by anyone, anywhere in your code. You didn't detail the usage of this property.
  • If it's the logon form that should update it and it will never update again, have your Program class set this field.

  • If it's another class that updates this field, maybe this property doesn't belong here.

  1. You have some code that is duplicated, so there must be a way to do better.

  2. From my point view, you are doing many different things in your Main method, maybe you could put these operations in separate methods : Init, Login, Start.

  3. Use using

  4. You put the Presenter for MainForm in the Tag Property, which I think belongs to a specific property. That way the MainForm can control who has access to it, otherwise any object that has an access to the MainForm can recover the Tag and therefore the Presenter. Another problem is that the Tag property is an object, so you will have to convert it every time you want to use it.

So, all together that would become :

using System;
using System.Windows.Forms;
using WindowsFormsApp.Presenters;
using WindowsFormsApp.Views;
using Autofac;
using DbContexts;
using Serilog;

namespace WindowsFormsApp
{
    internal static class Program
    {
        public static IContainer Container { get; private set; }
        public static string UserName { get; private set; }

        /// <summary>
        /// The main entry point for the application.
        /// </summary>
        [STAThread]
        private static void Main()
        {
            Init();
            Login();
            Start();
        }

        private void Init()
        {
            var builder = new ContainerBuilder();
            builder.Register(c => new MyContext());            
            Container = builder.Build();

            Log.Logger = new LoggerConfiguration()
                .MinimumLevel.Debug()
                .WriteTo.Console()
                .WriteTo.RollingFile("log-{Date}.txt")
                .CreateLogger();
            Log.Information("Application Started");

            Application.EnableVisualStyles();
            Application.SetCompatibleTextRenderingDefault(false);
        }

        private void Login()
        {
            using (loginForm = new LoginForm())
            {
                var results DialogResult.None;
                do
                {
                    results = loginForm.ShowDialog();
                    if (results == DialogResult.Cancel)
                        System.Environment.Exit(1);
                } while (results != DialogResult.OK);
                //Since we logged on correctly, we can update UserName (I guess)
                UserName = loginForm.ValidatedUserName;
            }
        }
 
        private void Start()
        {
            using (var mainFormView = new MainFormView())
            {
                mainFormView.Presenter = new MainFormPresenter(mainFormView);
                Application.Run(mainFormView);
            }
        }
    }
}
| improve this answer | |
\$\endgroup\$
2
\$\begingroup\$

The first thing I noticed. Code duplication. Whenever you see this your first thought should be,"There has to be a better way.".

In this case assign the result variable to DialogResult.None then just show the dialog inside the loop:

var result = DialogResult.None;
while(result != DialogResult.OK)
{
    result = loginForm.ShowDialog();
    if(result == DialogResult.Cancel)
    {
        System.Environment.Exit(1);
    }
}
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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