I made a password generator and I think my code isn´t the best readable. Are there any better ways to do it?

lukas9669

Member
Joined
Feb 7, 2022
Messages
9
Programming Experience
1-3
This is just the UI. The logic is fine.

C#:
static void Main(string[] args)
        {
            Console.WriteLine("This is a passwordgenerator.");
            Console.WriteLine("Enter the length of your password (12 signs recommended): ");
            int length;
            try
            {
                length = int.Parse(Console.ReadLine());
            }
            catch (Exception)
            {

                Console.WriteLine("Invalid input. The program will generate a password with 12 signs.");
                length = 12;
            }

            Console.WriteLine("Uppercase letters? (y/n): ");
            char upperCase;
            try { upperCase = char.Parse(Console.ReadLine()); } catch (Exception) { Console.WriteLine("Invalid input. Program will use (y)"); upperCase = 'y'; }
            bool containsUpperCase;
            if (upperCase == 'y') { containsUpperCase = true; } else if (upperCase == 'n') { containsUpperCase = false; } else { containsUpperCase = false; Console.WriteLine("Invalid input. Program will use (y)"); }

            Console.WriteLine("Other signs? (y/n): ");
            char sign;
            try { sign = char.Parse(Console.ReadLine()); } catch (Exception) { Console.WriteLine("Invalid input. Program will use (y)"); sign = 'y'; }
            bool containsSign;
            if (sign == 'y') { containsSign = true; } else if (sign == 'n') { containsSign = false; } else { containsSign = false; Console.WriteLine("Invalid input. Program will use (y)"); }

            Console.WriteLine("Numbers? (y/n): ");
            char number;
            try { number = char.Parse(Console.ReadLine()); } catch (Exception) { Console.WriteLine("Invalid input. Program will use (y)"); number = 'y'; }
            bool containsNumber;
            if (number == 'y') { containsNumber = true; } else if (number == 'n') { containsNumber = false; } else { containsNumber = false; Console.WriteLine("Invalid input. Program will use (y)"); }
        }
 
Yes.

Follow the .NET Framework naming recommendations.

Format your code using Allman indent style.

Move common code into methods.

Don't use exceptions for flow control. In your case, you should use TryParse() instead of Convert wrapped in try-catch.

If you must use try-catch don't do Pokemon exception handling (e.g. catch-em-all). Catch only the exceptions that you know you can handle. Let other exceptions bubble up (or terminate your app).
 
And if you must do catch-them-all exception handling at all, at the very least display the name of the exception so that you are sure what went wrong. Just as a matter of principle.
 
Realizing this is a month old question thought it is still worth posting the following example which is done with .NET Core, C#9 which will need modifications if using .NET Framework.

To keep code clean and concise, consider using NuGet package SnippetSpectre.Console.
  • Collecting user input with various forms of validation is done with TextPrompt class which is generics so you can keep code consistent.
  • Easy to get multiple choices using MultiSelectionPrompt
  • Supports various colors dependent on the environment

Model:
Snippetpublic class UserChoices
{

    public int PasswordLength { get; set; }

    public bool UseUppercaseLetters { get; set; }

    public bool UseOtherSigns { get; set; }

    public bool UseNumbers { get; set; }
}

A class responsible for prompting for input

Prompts:
public class Prompts
{
    public static int PasswordLength() =>
        AnsiConsole.Prompt(
            new TextPrompt<int>("Enter the length of your password ([b]12 signs recommended[/]):")
                .PromptStyle("yellow")
                .DefaultValue(12)
                .DefaultValueStyle(new Style(Color.Aqua))
                .ValidationErrorMessage("[red]Must be integer[/]")
                .Validate(value => value switch
                {
                    <= 0 => ValidationResult.Error("[red]1 is min value[/]"),
                    >= 12 => ValidationResult.Error("[red]12 is max value[/]"),
                    _ => ValidationResult.Success(),
                }));

    public static List<string> _choicesList => new List<string>()
    {
        "Uppercase letters?",
        "Other signs?",
        "Numbers?",
    };

    public static List<string> Choices() => AnsiConsole.Prompt
    (
        new MultiSelectionPrompt<string>()
            .Required(false)
            .Title("[cyan]Options[/]?")
            .InstructionsText("[grey](Press [yellow]<space>[/] to toggle a choice, [yellow]<enter>[/] to accept)[/] or [red]Enter[/] w/o any selections to cancel")
            .AddChoices(_choicesList)
            .HighlightStyle(new Style(Color.White, Color.Black, Decoration.Invert))
    );
}

Get choices

Collect information:
public class Configuration
{
    public static UserChoices Get()
    {
        UserChoices choices = new UserChoices
        {
            PasswordLength = Prompts.PasswordLength()
        };

        var options = Prompts.Choices();

        if (options.Count <= 0) return choices;

        choices.UseNumbers = options.Contains("Numbers");
        choices.UseOtherSigns = options.Contains("Other signs?");
        choices.UseNumbers = options.Contains("Numbers?");
        choices.UseUppercaseLetters = options.Contains("Uppercase letters?");

        return choices;
    }
}

Usage

Ask:
partial class Program
{
    static void Main(string[] args)
    {
        UserChoices userChoices = Configuration.Get();
        Console.ReadLine();
    }
}

screenShot.png


Full source
 
Back
Top Bottom