Search

Categories

 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 

Send mail to the author(s) E-mail

# Tuesday, 05 November 2013

image

BusinessLogic.Speaker.Register

image
Some code is 8 levels deep in.. goal is 3 levels.

image

Right click on BusinessLogic layer and do Code Analysis.

Method has 34 distinct paths through it (Cyclomatic complexity)
13 different classes are being called by this method
Only 53 actual lines of code (150 lines including whitespace and comments)

Refactoring

We have a test suite.

  • Zombie code get rid of
  • Mayfly variables – just in time.. move variables to just where they need to be
  • Remove guard clause checks to another private method
            if (!string.IsNullOrWhiteSpace(FirstName))
            {
                if (!string.IsNullOrWhiteSpace(LastName))
                {
                    if (!string.IsNullOrWhiteSpace(Email))
                    {
private void ValidateData()
        {
            if (string.IsNullOrEmpty(FirstName)) throw new ArgumentNullException("First Name Required");
            if (string.IsNullOrEmpty(LastName)) throw new ArgumentNullException("Last Name Required");
            if (string.IsNullOrEmpty(Email)) throw new ArgumentNullException("Email Required");
        }

Replaced if statements with a method call to ValidateData

        private bool AppearsExceptional()
        {
            if (YearsExperience > 10) return true;
            if (HasBlog) return true;
            if (Certifications.Count() > 3) return true;

            var preferredEmployers = new List<string> { "Microsoft", "Google", "Fog Creek Software", "37Signals" };
            if (preferredEmployers.Contains(Employer)) return true;
            return false;
        }
  • Returning early
  • Moving more checks into ValidateData
public int? Register(IRepository repository)
        {
            ValidateRegistration();

            int? speakerId;

            speakerId = repository.SaveSpeaker(this);

            //if we got this far, the speaker is registered.
            return speakerId;
        }

        private void ValidateRegistration()
        {
            ValidateData();

            bool speakerAppearsQualified = AppearsExceptional() || !ObviousRedFlags();
            if (!speakerAppearsQualified)
            {
                throw new SpeakerDoesntMeetRequirementsException("This speaker doesn't meet our standards.");
            }

            ApproveSessions();
        }

        private void ApproveSessions()
        {
            foreach (var session in Sessions)
            {
                session.Approved = !SessionIsAboutOldTecnology(session);
            }

            bool noSessionsApproved = Sessions.Count(s => s.Approved) == 0;
            if (noSessionsApproved) throw new NoSessionsApprovedException("No sessions approved.");
        }

        private bool SessionIsAboutOldTecnology(Session session)
        {
            var oldTechnologies = new List<string> { "Cobol", "Punch Cards", "Commodore", "VBScript" };

            foreach (var tech in oldTechnologies)
            {
                if (session.Title.Contains(tech) || session.Description.Contains(tech)) return true;
            }
            return false;
        }

        private bool ObviousRedFlags()
        {
            //need to get just the domain from the email
            string emailDomain = Email.Split('@').Last();

            var ancientEmailDomains = new List<string> { "aol.com", "hotmail.com", "prodigy.com", "CompuServe.com" };

            return (ancientEmailDomains.Contains(emailDomain) && ((Browser.Name == WebBrowser.BrowserName.InternetExplorer && Browser.MajorVersion < 9)));
        }

        private bool AppearsExceptional()
        {
            if (YearsExperience > 10) return true;
            if (HasBlog) return true;
            if (Certifications.Count() > 3) return true;

            var preferredEmployers = new List<string> { "Microsoft", "Google", "Fog Creek Software", "37Signals" };
            if (preferredEmployers.Contains(Employer)) return true;
            return false;
        }

        private void ValidateData()
        {
            if (string.IsNullOrEmpty(FirstName)) throw new ArgumentNullException("First Name Required");
            if (string.IsNullOrEmpty(LastName)) throw new ArgumentNullException("Last Name Required");
            if (string.IsNullOrEmpty(Email)) throw new ArgumentNullException("Email Required");
            if (Sessions.Count() == 0) throw new ArgumentException("Can't register speaker with no sessions to present.");
        }

Refactoring to small Methods that do 1 thing!

image
75 – better!

image

image

| | # 

image
Agreed – R# can give suggestions to this.. eg var… () are superfluous.. linq the foreach (I like foreach though).  .Sum extension methods.
More descriptive names!

Naming Classes

  • Noun (a thing)
  • Specific as possible
  • Single responsibility
  • Avoid generic suffixes eg ProductInfo, ProductManager..confusing

Method Names

image

Naming Booleans

image

| | # 

http://pluralsight.com/training/Courses/TableOfContents/writing-clean-code-humans

image

Conventions

Dirty vs Clean code

image

image

image
Great point here on Linq-to-SQL

TED

  • Terse
  • Expressive
  • Do One Thing

image
7 items in brain at once +- 2.

image

image
Layers of abstraction so can walk through the code at different levels of detail.

| | #