I am new to C# and develop a configurable PasswordService class. As the code works the next step is to improve the code.Right now there are the following options:Set alpha characters and the number of alpha characters that has to be in the password.Set numeric and the number of numeric characters that has to be in the password.Set non alphanumeric and the number of non alphanumeric characters that has to be in the password.My questions:Is the structure fine as it is?Are the thrown Exceptions and it's message well chosen?What do you think about the generate function yet?Would you add or adjust the options?Are the comments c#/xml valid?How can I improve the password algorithm?Did I miss something else?using System;using System.Collections.Generic;using System.Linq;using System.Text;using System.Threading.Tasks;namespace PasswordGenerator{namespace Utility{namespace Service{    /// <summary>    /// This is a configurable password generator.    /// </summary>    class PasswordService    {        string sAlphaCharacters;        string sNumericCharacters;        string sNonAlphanumericCharacters;        uint uiNumberOfAlphaCharacters;        uint uiNumberOfNumericCharacters;        uint uiNumberOfNonAlphanumericCharacters;        /// <summary>        /// Initializes class with common configuration.        /// </summary>        public PasswordService()        {            this.SAlphaCharacters = abcdefghijklmnopqrstuwvxyABCDEFGHIJKLMNOPQRSTUWVXYYZ;            this.SNumericCharacters = 0123456789;            this.SNonAlphanumericCharacters = !$%&()[]{}=?#;            this.UiNumberOfAlphaCharacters = 6;            this.UiNumberOfNumericCharacters = 6;            this.UiNumberOfNonAlphanumericCharacters = 3;        }        // Accessor & modifier        public string SAlphaCharacters        {            get            {                return sAlphaCharacters;            }            set            {                System.Text.RegularExpressions.Regex oRegex = new System.Text.RegularExpressions.Regex([^a-zA-Z]{1,});                if (oRegex.IsMatch(value))                {                    throw new ArgumentException(Value must not contain anything but alpha characters., SAlphaCharacters.set);                }                sAlphaCharacters = value;            }        }        public string SNumericCharacters        {            get            {                return sNumericCharacters;            }            set            {                System.Text.RegularExpressions.Regex oRegex = new System.Text.RegularExpressions.Regex([^0-9]{1,});                if (oRegex.IsMatch(value))                {                    throw new ArgumentException(Value must not contain anything but numeric characters., SNumericCharacters.set);                }                sNumericCharacters = value;            }        }        public string SNonAlphanumericCharacters        {            get            {                return sNonAlphanumericCharacters;            }            set            {                System.Text.RegularExpressions.Regex oRegex = new System.Text.RegularExpressions.Regex([a-zA-Z0-9]{1,});                if (oRegex.IsMatch(value))                {                    throw new ArgumentException(Value must not contain alphanumeric characters., SNonAlphanumericCharacters.set);                }                sNonAlphanumericCharacters = value;            }        }        public uint UiNumberOfAlphaCharacters        {            get            {                return uiNumberOfAlphaCharacters;            }            set            {                uiNumberOfAlphaCharacters = value;            }        }        public uint UiNumberOfNumericCharacters        {            get            {                return uiNumberOfNumericCharacters;            }            set            {                uiNumberOfNumericCharacters = value;            }        }        public uint UiNumberOfNonAlphanumericCharacters        {            get            {                return uiNumberOfNonAlphanumericCharacters;            }            set            {                uiNumberOfNonAlphanumericCharacters = value;            }        }        /// <summary>        /// Chooses randomly characters of given group and returns them.        /// </summary>        /// <param name=sCharacterGroup>From this character group the chararacters will be choosen.</param>        /// <param name=uiNumberOfThisGroup>The amount of characters to choose randomly.</param>        /// <returns>One random character of the character group</returns>        public string getRandomSubstrOfCharacterGroup(string sCharacterGroup, uint uiNumberOfThisGroup)        {            Random rnd = new Random();            int iPosition = 0;            string sCharacters = ;            for (uint ui = 0; ui < uiNumberOfThisGroup; ui++)            {                iPosition = iPosition = rnd.Next(0, sCharacterGroup.Length);                sCharacters += sCharacterGroup.ToCharArray()[iPosition].ToString();            }            return sCharacters;        }        /// <summary>        /// Based upon the config a password will be generated and returned.        /// </summary>        /// <exception cref=InvalidOperationException>        /// Thrown when the number of each character group is 0.        /// </exception>        /// <returns>        /// Returns generated password.        /// </returns>        public string generate()        {            if (this.UiNumberOfAlphaCharacters == 0 && this.UiNumberOfNumericCharacters == 0 && this.UiNumberOfNonAlphanumericCharacters == 0)            {                throw new InvalidOperationException(The number of each character group is 0.);            }            Random oRand = new Random();            string sPassword = ;            sPassword += this.getRandomSubstrOfCharacterGroup(this.SAlphaCharacters, this.uiNumberOfAlphaCharacters);            sPassword += this.getRandomSubstrOfCharacterGroup(this.SNumericCharacters, this.UiNumberOfNumericCharacters);            sPassword += this.getRandomSubstrOfCharacterGroup(this.SNonAlphanumericCharacters, this.UiNumberOfNonAlphanumericCharacters);            sPassword = new string(sPassword.ToCharArray().OrderBy(s => (oRand.Next(2) % 2) == 0).ToArray());            return sPassword;        }    }}}}
C# PasswordService class
c#;error handling;exception
I covered the security problem in another answer; in this answer let's look at more subtle problems.Someone already mentioned that the nested namespaces are unnecessarily verbose, but even with the more compact syntax, things are still not great. We have a class PasswordGenerator.Utility.Service.PasswordService. What is the function of each portion of the namespace?  Namespaces are used primarily to organize large bodies of code, and secondarily to mitigate the danger of collisions between your password service and another.  So, first, when I see your namespace there with its three levels I assume that there must be hundreds or thousands of classes that you are organizing, that there is a PasswordGenerator.Utility.Foo and PasswordGenerator.Utility.Bar and PasswordGenerator.Blah.Whatever as well -- in short, that three levels are necessary to organize the mass of code you have in there.The standard way to structure a namespace is Organization.FunctionalArea.SubArea -- so, Microsoft.CodeAnalysis.CSharp, for example. The namespace you've come up with seems there only to hold a single class which does exactly what the namespace name says it does. I would be inclined to have a single namespace PasswordUtilities and be done with it.The name PasswordService is vague. What kind of service?  Generation? Verification? Entropy measuring?  The class relies upon its namespace to make it clear that it is a generator. That's completely backwards. The namespace should be PasswordUtilities and the class should be Generator.Others have already commented on your use of System Hungarian, which Joel and I both noted in 2003 is the bad Hungarian. Neither form of Hungarian should be used in C#.Do not use unsigned integers in C# even for quantities that are logically non-negative. String.Length is signed. Follow its example.Your test for whether a character is alpha is deeply prejudiced towards American usage. If a Greek person wishes their password to be   shouldn't they be allowed to do so?  If you want to check for alpha-ness, use the methods on char that do so.Follow C# naming conventions public string getRandomSubstrOfCharacterGroupOthers have noted that methods begin with a capital letter in C#.Worse: mthds shd nt cntn abbrvs, u r totes nt a 13yo grl wrtng nts n clss.Why is this method public? It seems to be an implementation detail.The methods name is incorrect. It does not return a random substring.  CAB is not a substring of ABCDEFG. CAB is a substring of TAXICAB and CABERNET. sCharacters += sCharacterGroup.ToCharArray()[iPosition].ToString();The ToString is unnecessary. C# automatically calls ToString on your behalf when appending to a string.Others have suggested that StringBuilder would be the better choice here. If the string were likely to be hundreds or thousands of characters, then yes, always use a builder. But for this little thing that is only six or ten characters, no need to worry about it.        sPassword = new string(sPassword.ToCharArray().OrderBy(s => (oRand.Next(2) % 2) == 0).ToArray());I cannot for the life of me figure out what this thing is trying to do. It looks like you're trying to shuffle the string, but why are you shuffling it by flipping a coin (Next(2)) then making sure it was either heads or tails (the remainder), then turning the 0 or 1 into true or false, and then sorting the string by heads first?  This is a bizarre way to shuffle a string. Can you explain what your logic was here?  Maybe there is some good reason for this, and if there is, it should be very clearly explained in the comments.If I wanted to shuffle a string like this then I'd say         password = new string(password.ToCharArray().OrderBy(s => random.NextDouble()).ToArray());That is generate a random number between 0 and 1 as the sort key.  System.Text.RegularExpressions.Regex oRegex = new System.Text.RegularExpressions.Regex([a-zA-Z0-9]{1,});Notice how completely uninteresting information dominates this statement. Use a using System.Text.RegularExpressions; directive to bring Regex into scope, and then use var to simplify the declaration: var regex = new Regex([a-zA-Z0-9]{1,});