A modern architecture review
This presentation is the property of its rightful owner.
Sponsored Links
1 / 128

A Modern Architecture Review PowerPoint PPT Presentation


  • 129 Views
  • Uploaded on
  • Presentation posted in: General

A Modern Architecture Review. Using Code Review Tools. @ AdamCogan | # vsalm #tee12 #dev324 . Agenda. #1 The 1 st things to look out for Processes Does it work? Documentation #2 High Level tools Architecture Code Analysis Code Metrics #3 Manual Checking SOLID Design Principles

Download Presentation

A Modern Architecture Review

An Image/Link below is provided (as is) to download presentation

Download Policy: Content on the Website is provided to you AS IS for your information and personal use and may not be sold / licensed / shared on other websites without getting consent from its author.While downloading, if for some reason you are not able to download a presentation, the publisher may have deleted the file from their server.


- - - - - - - - - - - - - - - - - - - - - - - - - - E N D - - - - - - - - - - - - - - - - - - - - - - - - - -

Presentation Transcript


A modern architecture review

A Modern Architecture Review

Using Code Review Tools

@AdamCogan | #vsalm #tee12 #dev324

Delivering Awesome Web Applications


Agenda

Agenda

#1 The 1st things to look out for

  • Processes

  • Does it work?

  • Documentation

    #2 High Level tools

  • Architecture

  • Code Analysis

  • Code Metrics

    #3 Manual Checking

  • SOLID Design Principles

  • Code Review tools


About adam

About Adam

  • Chief Architect at SSW

    • Developing custom solutions for businesses across a range of industries such as Government, banking, insurance

  • Microsoft Gold Partner

  • Microsoft Regional Director

  • VSTS MVP

  • @AdamCogan


A modern architecture review

I Believe


The first things to look out for

The first things to look out for


Do you evaluate the processes

Do you evaluate the processes?

  • Often times this is the source of problems

    • Are devs getting bogged down in the UI?

    • Do you have a Scrum Master?

    • Do you have continuous integration?

    • Do you have continuous deployment?

    • Do you have a Schema Master?

    • Do you have a TFS Master?

http://rules.ssw.com.au/SoftwareDevelopment/RulestobetterArchitectureandCodeReview/Pages/DoYouHaveTheDesignersFixingUpTheUI.aspx


Are they on the latest version

Are they on the latest version?

  • …of VS

  • …of TFS

  • Alternatively for a ‘Gold Star’ …

    • Resharper, add-ins, extensions

http://rules.ssw.com.au/SoftwareDevelopment/RulestobetterArchitectureandCodeReview/Pages/CanYouGetLatestAndCompile.aspx


Can you get latest and compile l1

Can you ‘Get latest’ and compile? #L1

  • It's amazing how often you can't simply do a "Get Latest" and compile

  • Add _Instructions_Compile.docx

  • Then run unit tests…

http://rules.ssw.com.au/SoftwareDevelopment/RulestobetterArchitectureandCodeReview/Pages/CanYouGetLatestAndCompile.aspx


Can you get latest and compile l11

Can you get latest and compile? #L1

  • See the Integration.Test project fail

  • Add _Instructions_Compile.docx

http://rules.ssw.com.au/SoftwareDevelopment/RulestobetterArchitectureandCodeReview/Pages/CanYouGetLatestAndCompile.aspx


Can you get latest and compile l2

Can you get latest and compile? #L2

  • Creation of the database via scripts (incremental)Tip: use OSQL, SQLCMD or SSW SQL Deploy

  • and re-run the tests. They will now pass.


Do you want a gold star l3

Do you want a ‘Gold Star’ ? #L3

  • Streamline setup of a new development environment

  • Problems to check for:

    • Windows 8 not supported

    • Many things to build

    • Lots of dependencies

  • Recommendation: All manual work station setup steps should be scripted with PowerShell

  • Recommendation: A get + compile should work within 1 minute, and work without a dev being on the domain (to support external consultants)


Figure you see the problems in the devs environment note prefix eg 01setup environment ps1

Figure: You see the problems in the devsenvironmentNote Prefix eg. _01Setup-Environment.ps1

PS C:\Code\Northwind> .\Setup-Environment.ps1

Problem: Azure environment variable run state directory is not configured (_CSRUN_STATE_DIRECTORY).

Problem: Azure Storage Service is not running. Launch the development fabric by starting the solution.

WARNING: Abandoning remainder of script due to critical failures.

To try and automatically resolve the problems found, re-run the script with a -Fix flag.


Figure the script tries to automatically fix the problems

Figure: The script tries to automatically fix the problems

PS C:\Code\Northwind> .\Setup-Environment.ps1 -fix

Problem: Azure environment variable run state directory is not configured (_CSRUN_STATE_DIRECTORY).

Fixed: _CSRUN_STATE_DIRECTORY user variable set

Problem: Azure Storage Service is not running. Launch the development fabric by starting the solution.

WARNING: No automated fix available for 'Azure Storage Service is running'

WARNING: Abandoning remainder of script due to critical failures.


Figure note on 2 nd run only 1 script remains see there is less to fix

Figure: Note on 2nd run only 1 script remains – see there is less to fix

PS C:\Code\Northwind> .\Setup-Environment.ps1 -fix

Problem: Azure Storage Service is not running. Launch the development fabric by starting the solution.

WARNING: No automated fix available for 'Azure Storage Service is running'

WARNING: Abandoning remainder of script due to critical failures.


Can you check in and deploy l1

Can you ‘Check In’ and Deploy #L1

  • Add _Instructions_Deploy.docx

  • Alternatively for a ‘Star’ …

http://rules.ssw.com.au/SoftwareDevelopment/RulestobetterArchitectureandCodeReview/Pages/Default.aspx


Can you check in and deploy l2

Can you ‘Check In’ and Deploy #L2

  • Use PowerShell scripts as your documentation

    • build.ps1

    • deploy_dev.ps1

    • deploy_test.ps1

    • deploy_prod.ps1

  • Alternatively for a ‘Gold Star’ … TFSBuild + Portal

http://rules.ssw.com.au/SoftwareDevelopment/RulestobetterArchitectureandCodeReview/Pages/Default.aspx


Can you check in and deploy l2 tfsbuild portal

Can you ‘Check In’ and Deploy #L2TFSBuild + Portal


Do you review the solution and project names

Do you review the Solution and Project names?

  • The name of your solution and the names of the project in your solution should be consistent

http://rules.ssw.com.au/SoftwareDevelopment/RulestobetterArchitectureandCodeReview/Pages/DoYouReviewTheSolutionName.aspx


Do you review the documentation

Do you review the documentation?

  • Old School:

    • Heavy, long documents

    • Sequence Diagrams

    • UML

http://rules.ssw.com.au/SoftwareDevelopment/RulestobetterArchitectureandCodeReview/Pages/DoYouReviewTheDocumentation.aspx


Suggestions for doco eg enterprise architect

Suggestions for doco eg. Enterprise Architect

  • Use Red for unimplemented stuff

  • Use the DateTime shape To see the last time the diagram was modified and by whom

  • Mark items as ‘TODO: Adam’. For items still pending


Do you review the documentation1

Do you review the documentation

  • New School:

    • 4 docs

      • Business.docx

      • _Instructions_Compile.docx

      • _Instructions_Deploy.docx

      • Technologies.docx

    • Unit Tests (low level)

    • Code and Work Items (low level  PBI)


A modern architecture review

Vote:


High level tools

High Level Tools


Agenda1

Agenda

#1 The 1st things to look out for

  • Processes

  • Does it work?

  • Documentation

    #2 High Level tools

  • Architecture

  • Code Analysis

  • Code Metrics

    #3 Manual Checking

  • SOLID Design Principles

  • Code Review tools


Do you look at the architecture

Do you look at the architecture?

  • 2 Choices:

    • VS Dependency Graph or … ?

      • Ultimate Edition

    • nDepend?

      • 3rd Party Tool for a few hundred $

      • Weak dependency graph

      • No need for VS Ultimate

      • Bonus: See problems in the ‘Queries + Rules Explorer’

http://rules.ssw.com.au/SoftwareDevelopment/RulestobetterArchitectureandCodeReview/Pages/DoYouLookAtTheArchitecture.aspx


My dream

My Dream


My dream instead of this

My Dream – instead of this


My dream it automatically does this

My Dream – it automatically does this


A modern architecture review

Great Overview tool and learning toolNot for day-to-day use as a code analysis tool.Note: Don’t be distracted by a colour problem 


Do you use the best code analysis tools

Do you use the best Code Analysis tools?

http://rules.ssw.com.au/SoftwareDevelopment/RulestobetterArchitectureandCodeReview/Pages/DoYouDoCodeAnalysis.aspx


Do you use the best code analysis tools1

Do you use the best Code Analysis tools?

  • Level 1 – ReSharper – All Rules

  • Level 2 - VS Code Analysis (FXCop) – Default Settings

  • Level 3 - VS Code Analysis (FXCop) – All Rules

  • Level 4 – StyleCop – All Rules

  • Level 5 – SSW Code Auditor – All Rules?

http://rules.ssw.com.au/SoftwareDevelopment/RulestobetterArchitectureandCodeReview/Pages/DoYouDoCodeAnalysis.aspx


Do you use the best code analysis tools2

Do you use the best Code Analysis tools?

http://rules.ssw.com.au/SoftwareDevelopment/RulestobetterArchitectureandCodeReview/Pages/DoYouDoCodeAnalysis.aspx


Do you use the best code analysis tools3

Do you use the best Code Analysis tools?

  • Level 1 – ReSharper – campsite scout rule

  • Level 2 - VS Code Analysis (FXCop) – Default Settings

  • Level 3 - VS Code Analysis (FXCop) – Custom

  • Level 4 – StyleCop

  • Level 4 – StyleCop - Custom

  • Level 5 – SSW Code Auditor

  • Level 5 – SSW Code Auditor - Custom

  • TIP: Have a document with rules that you turn off and the reason

http://rules.ssw.com.au/SoftwareDevelopment/RulestobetterArchitectureandCodeReview/Pages/DoYouDoCodeAnalysis.aspx


Resharper

ReSharper


Resharper custom

Resharper Custom


Code analysis

Code Analysis


Code analysis custom

Code Analysis Custom


Code analysis suppress 1 add in suppression file or in code

Code Analysis – Suppress #1Add in ‘Suppression File’ or in code?

  • ?


Code analysis suppress 2

Code Analysis – Suppress #2

Add in ‘Suppression File’ or in code?

  • Rule No Good – removed from Rule Set

  • Rule N/A in this case – in ‘Suppression File’

  • Rule is Valid – in this case I am overriding it


Code analysis create work item 1 add as bug pbi or task

Code Analysis – Create work item #1Add as Bug, PBI or Task...

?


Code analysis create work item 2

Code Analysis – Create work item #2

Neno’s solution = Select 30 in a PBI

Leave as warning

Aussie solution = 1 becomes a PBI


Stylecop

StyleCop


Code auditor

Code Auditor


Rules turned off

Rules turned off


Do you check the code coverage

Do you check the code coverage?

  • See if there are unit tests

  • See if they are any good (~ 80% coverage)

http://rules.ssw.com.au/SoftwareDevelopment/RulestobetterArchitectureandCodeReview/Pages/DoYouLookForCodeCoverage.aspx


Report time

Report time?

  • If you are doing a report, gather some more reports…

    Eg. Code Metrics, Code Coverage, Code Clones

  • But lets assume we are continuing to ‘Probe’… So from:

    • SRP

    • Naming Conventions


Our solution is clean now what next

Our solution is clean now. What next?


Agenda2

Agenda

#1 The 1st things to look out for

  • Processes

  • Does it work?

  • Documentation

    #2 High Level tools

  • Architecture

  • Code Analysis

  • Code Metrics

    #3 Manual Checking

  • SOLID Design Principles

  • Code Review tools


Do you run code metrics to find dodgy code

Do you run Code Metrics to find dodgy code?

  • Use the “Hot Spots” feature to quickly identify smelly code

  • It measures:

    • Maintainability Index

    • Cyclomatic Complexity

    • Depth of Inheritance

    • Class Coupling

    • Lines of Code


Code metrics results

Code Metrics Results


Manual checking getting our hands dirty

Manual Checking:Getting our hands dirty


Manual review

Manual Review

  • After using the automated high level tools it’s time to actually jump into the code

  • Look for code that doesn’t follow SOLID principles… and then design patterns

  • Speak to the devs


Solid principles

SOLID Principles

  • Single Responsibility Principle

  • Open Close Principle

  • Liskov Substitution Principle

  • Interface Segregation Principle

  • Dependency Inversion Principle

http://rules.ssw.com.au/SoftwareDevelopment/RulestobetterArchitectureandCodeReview/Pages/DoYouKnowCommonDesignPrinciples.aspx


Single responsibility principle

Single Responsibility Principle

  • A class should have one and only one responsibility

  • public class Address { // Standard Address Propertiespublic Image GetGoogleMaps() {}public decimal GetDistance(Address destination) {}public boolValidateAddress() {}}


Single responsibility principle1

Single Responsibility Principle

  • The Address class has too many responsibilities

    • Showing an image of the address (tied to UI)

    • Calculations based on the address

    • Validation of the address

  • Another UI may use BingMaps instead of Google Maps

  • Some, if not all of these functions should be moved to other classes like MapHelperand AddressHelper


Single responsibility principle2

Single Responsibility Principle

  • public class AddressHelper{public decimal GetDistance(Addressstart, Address destination) {}public boolValidateAddress(Addressaddress) {}}

  • public class GoogleMapProvider : IMapProvider{public ImageGetMap(Address start) {}}public class BingMapProvider: IMapProvider{public ImageGetMap(Address start) {}}


Open closed principle

Open Closed Principle

  • Open for extension, but closed for modification

  • Use Abstract base classes that specify some base functionality

  • E.g. In .NET WebRequestallows you to connect servers via web protocols

    • HttpWebRequest

    • FtpWebRequest

    • FileWebRequest


Liskov s substitution principle

Liskov’s Substitution Principle

  • Subtypes must be substitutable for their base types

    public abstract class Duck {public abstract void Quack();}


Liskov s substitution principle1

Liskov’s Substitution Principle

public class PekinDuck: Duck {public void Quack() {}} public class BatteryPoweredDuck: Duck{publicBatteryPoweredDuck(Battery energizer) {}public void Quack() {}}

  • Can’t swap PekinDuckwith BatteryPoweredDuckbecause you need pass it some batteries first


Another example

Another example

public class Rectangle {protected int _width; protected int_height; public intWidth { get { return _width; }public intHeight { get { return_height; }public void SetWidth(int width) { _width = width; }public void SetHeight(int height) { _height = height; }}

public class Square : Rectangle { public override voidSetWidth(int width) { _width = width; _height = width; } public override void SetHeight(int height) { _width = height; _height = height; }}


Another example cont

Another Example (cont…)

  • If we try to use these classes

    varshape = newRectangle();shape.SetWidth(2);shape.SetHeight(5);Console.WriteLine(String.Format(“Area = {0}”, shape.Height * shape.Width))// Prints 10 as expected (2 * 5 = 10)


Another example cont1

Another Example (cont…)

  • If we try the Square

    varshape = newSquare();shape.SetWidth(2);shape.SetHeight(5);Console.WriteLine(String.Format(“Area = {0}”, shape.Height* shape.Width))// Prints 25 not as expected since we set the Width = 2 and Height = 5


Another example cont2

Another Example (cont..)

  • The Square violates the Liskov substitution principle as we don’t get expected behaviour as setting the width will also modify the height and vice versa


Interface segregation principle

Interface Segregation Principle

  • Don’t create interfaces with lots of methods that don’t necessarily get used in their implementations

public class MockingBird: IBird{public void Chirp() {}public void Flap() {}public void Fly() {}public void Eat() {}}

public interface IBird{public void Chirp();public void Flap();public void Fly();public void Eat();}


Interface segregation principle1

Interface Segregation Principle

public classEmu : IBird{public void Chirp() {}public void Flap() {}public void Fly() {} public void Eat() { throw new NotImplementedException(); }}


Interface segregation principle2

Interface Segregation Principle

public interface IBird{public void Chirp();public void Flap();public void Eat();}

public interface IFlyingBird: IBird{public void Fly();}

public class Emu : IBird{public void Chirp() {}public void Flap() {}public void Fly() {}}


A modern architecture review

Dependency Inversion PrincipleDepend on abstractions, not on implementations. Anything required to create a valid instance of an object, should have those dependencies passed in as arguments to the constructor

  • Higher level classes to depend on abstractions of lower level classes (swappable)

  • publicclassEmployee { publicEmployee() {FavCoffee = new Cappuccino();Skills=newList<Skills> { newMSAccessSkill(),newSharePointSkill(), newDotNetNukeSkill() } } }


Dependency inversion principle

Dependency Inversion Principle

  • publicclassEmployee { publicEmployee(IDrinkableiamthirsty, IEnumerable<Skills> skills) {} }


Dependency inversion principle1

Dependency Inversion Principle

varEricPhan = newEmployee(CoffeeFactory.Get(“Latte”), newList<Skills> { newBusinessIntelligenceSkill(),newMVC4Skill(),newDotNetSkill() });

varMarkLiu = newEmployee(CoffeeFactory.Get(“Skim Cappuccino”), newList<Skills> { newMSAccessSkill(),newSharePointSkill(), newDotNetNukeSkill() });


Do you know the common design patterns

Do you know the common Design Patterns?

  • ?

    “Communicate massive amount of data in a few words”

  • Adam Cogan

    “Accepted solutions to well known problems”

  • Ben Day

http://rules.ssw.com.au/SoftwareDevelopment/RulestobetterArchitectureandCodeReview/Pages/DoYouKnowCommonDesignPatterns.aspx


Do you know the common design patterns1

Do you know the common Design Patterns?

  • Inversion of Control

  • Dependency Injection

  • Factory

  • Singleton

  • Repository

  • Unit of Work

  • MVC

  • MVP

  • MVVM

  • Abstract factory

  • Adapter

  • Bridge

  • Mediator

  • Proxy

  • Flyweight

  • Chain of responsibility

  • Command

  • Memento

  • Iterator

  • Visitor

  • State

  • Composite

  • Facade

  • Observer

  • Decorator

  • Null object

  • Interpreter

http://rules.ssw.com.au/SoftwareDevelopment/RulestobetterArchitectureandCodeReview/Pages/DoYouKnowCommonDesignPatterns.aspx


Anti patterns

Anti-Patterns


Abstractness vs instability diagram

Abstractness vs Instability Diagram

  • Shows the assemblies that are painful to maintain i.econcrete and stable, and which assemblies are potentially uselessi.eabstract and instable

  • Abstract: The assembly contains many abstract types (i.e interfaces and abstract classes) and few concrete types

  • Stable: The assembly is considered stable if its types are used by a lot of types of tier assemblies. In this conditionstable means painful to modify.


Do you start reading code comments

Do you start reading code? Comments

  • Q:\ Good or Bad?

  • Comments are a smell

  • Includes comments that explain the intent (the why rather than the what)


Do you start reading code

Do you start reading code?

  • Is clear and easy to read

  • Has consistent and meaningful names for everything

  • Has no duplicate or redundant code

  • Has consistent styles and formatting

  • Explains "why" when you read down, and "how" when you read left to right

http://rules.ssw.com.au/SoftwareDevelopment/RulestobetterArchitectureandCodeReview/Pages/DoYouStartReadingCode.aspx


Do you know code reviews after check in are bad

Do you know Code Reviews after check in are bad?

  • If you are aiming to get the to nirvana of Continuous Deployment, then you cant *rely* on code reviews after the fact.

  • Code Reviews have different status:* Important* Nice to have


The scenario

The Scenario

  • Mark is migrating from DotNetNuke to MVC

  • He’s unsure of his code because he doesn’t know Razor

  • Add some code with a comment ‘this could be done better’

  • Checks in anyway


U se the code review tools in tfs 2012

Use the Code Review tools in TFS 2012

  • TFS 2012 has built in Code Review tools

  • Hooks into the check-in/shelving process

  • This allows code to be manually reviewed before checking in


Suspend current changes

Suspend current changes


Request a review

Request a Review


Assign reviewer

Assign Reviewer


Reviewer gets the request

Reviewer Gets the Request


Check out the changeset

Check out the changeset

Related work items

Modified files


A modern architecture review

Shows the changed code


A modern architecture review

Side by Side (Standard Diff)


A modern architecture review

#2

#1


Finalizing the code review

Finalizing the Code Review


Check the code review

Check the Code Review


Check any comments

Check any comments


View code marked for review

View code marked for review


Mark any flagged code as completed

Mark any flagged code as completed


Mark the code review as complete

Mark the Code Review as Complete


Code review summary

Code Review Summary

  • Today: Developer does some work and wants to get it checked before committing to source control

  • Suggestion to MS: Add a new scenario

    • No touching of code

    • Architect ‘Adds comments’ during code review

    • Automatically adds the developers who originally worked on that section (instead of manually using annotate)

    • Creates “Code Review” work items

http://www.ssw.com.au/ssw/Standards/BetterSoftwareSuggestions/TeamFoundationServer.aspx


Resources

Resources

  • Thanks: Eric Phan and Adam Stephensen

  • Thanks: Marcel De Vries and Terje Sandstrom

  • Google: sswtv architecture code reviews

  • http://rules.ssw.com.au/SoftwareDevelopment/RulestobetterArchitectureandCodeReview/


Summary

Summary

#1 The 1st things to look out for

  • Processes

  • Does it work?

  • Documentation

    #2 High Level tools

  • Architecture

  • Code Analysis

  • Code Metrics

    #3 Manual Checking

  • SOLID Design Principles

  • Code Review tools


Evaluations 2 things

Evaluations + 2 things

  • Include best tip or tool you heard. Plus your tip to me.

  • @[email protected]

  • Come and visit me (+ Sydney .NET User Group)


Thank you

Thank You!

Sydney | Melbourne | Brisbane | Adelaide

[email protected]

www.ssw.com.au


Questions

Questions?

Take a business card.


Refactoring

Refactoring

  • Level 1: Minimum

  • Level 2: Custom

  • Note: All disabled ones to be documentedeg. http://www.benday.com/2010/03/15/article-static-methods-are-a-code-smell/


Resources1

Resources

Learning

TechNet

  • Connect. Share. Discuss.

  • Microsoft Certification & Training Resources

http://europe.msteched.com

www.microsoft.com/learning

  • Resources for IT Professionals

  • Resources for Developers

http://microsoft.com/technet

http://microsoft.com/msdn


Submit your evals online

Evaluations

Submit your evals online

http://europe.msteched.com/sessions


  • Login