程序代写代做代考 SOFT2201/COMP9201: Software Design and Construction 1 – cscodehelp代写
SOFT2201/COMP9201: Software Design and Construction 1
Code Reviews
Dr Martin McGrane
School of Computer Science
The University of 1
Outline
– Why review
– What to review
– What is a review
– What is a formal review
– What is an informal review The University of 2
Why review?
The University of 3
Why review?
– What is the software, or aspect of the software, to be reviewed for?
– Whatarethespecificationsthatneedtobemet?
– Does the software, or aspect thereof, being reviewed,
meet those specifications?
The University of 4
What is the software for and does it work?
– What is the software, or aspect of the software, to be reviewed for?
– Whatarethespecificationsthatneedtobemet?
– Does the software, or aspect thereof, being reviewed,
meet those specifications?
– We need a way to check that the software is appropriately designed to meet the expected criteria
The University of 5
What to review?
The University of 6
What to review?
– Any part of a software project can be reviewed – Code
– Documentation – Processes
– Management
– Specifications – Planning
The University of 7
What is a review?
The University of 8
What is a review?
– A software review is
“A process or meeting during which a software product, set of software products, or a software process is presented to project personnel, managers, users, customers, user representatives, auditors or other interested parties for examination, comment or approval.”
IEEE Standard 1028-2008, “IEEE Standard for Software Reviews”, clause 3.5
The University of 9
Formal vs informal reviews
The University of 10
What is a formal review?
The University of 11
Fagan inspection
– An early effort to formalise the process of reviews
– The basis, or at least similar, to subsequent formal
approaches
– Describe program development process in terms of operations
– Define ‘entry’ and ‘exit’ criteria for all operation
– Specify objectives for the inspection process to keep
the team focused on one objective at a time
The University of 12
Fagan inspection
– Classify errors by type, rank by frequency, identify which types to spend most time looking for
– Analyse results and use for constant process improvement
The University of 13
Fagan inspection operation
– Specify objectives for the inspection process to keep the team focused on one objective at a time
– Planning
– Overview
– Preparation – Inspection – Rework
– Follow-up
The University of 14
Fagan inspection operations
– Planning
– Preparationofmaterials
– Arrangingofparticipants
– Arrangingofmeetingplace
– Overview
– Group education of participants on review materials – Assignmentofroles
The University of 15
Fagan inspection operations
– Preparation
– Participantsreviewitemtobeinspectedandsupporting material to prepare for the meeting, noting any questions or possible defects
– Participantspreparetheirroles – Inspectionmeeting
– Actual finding of defect
The University of 16
Fagan inspection operations
– Rework
– Rework is the step in software inspection in which the defects found during the inspection meeting are resolved by the author, designer or programmer. On the basis of the list of defects the low-level document is corrected until the requirements in the high-level document are met.
– Follow-up
– In the follow-up phase of software inspections all defects found in the inspection meeting should be corrected. The moderator is responsible for verifying that this is indeed the case. They should verify that all defects are fixed and no new defects are inserted while trying to fix the initial defects.
The University of 17
Formal inspection
– Managementreview
– Monitorprogress
– Determinestatusofplans
– Evaluate management effectiveness
– Supportsdecisionsaboutchangesindirection,resource allocation, and scope
The University of 18
Formal inspection
– Managementreview
– Maintenanceplans
– Disasterrecovery
– Migrationstrategies
– Customercomplaints
– Riskmanagementplans –…
The University of 19
Formal inspection
– Managementreviewroles – Decisionmaker
– Reviewleader
– Recorder
– Managementstaff – Technicalstaff
The University of 20
Formal inspection
– Managementreviewprocesses
– Preparation
– Plan time and resources
– Providefunding
– Providetraining
– Ensurereviewsareconducted
– Act on reviews
The University of 21
Formal inspection
– Technicalreview
– Softwarerequirements
– Softwaredesign
– Softwaretestdocumentation – Specifications
– …procedures
The University of 22
Formal inspection
– Technicalreviewroles – Decisionmaker
– Reviewleader
– Recorder
– Technicalreviewer
The University of 23
Formal inspection
– Inspections
– Thepurposeofaninspectionistodetectandidentifysoftware product anomalies. An inspection is a systematic peer examination that does one or more of the following:
• Verifies that the software product satisfies its specifications • Verifies that the software product exhibits specified quality
attributes
• Verifies that the software product conforms to applicable regulations, standards, guidelines, plans, specifications, and procedures
The University of 24
External Audits
– Reviews may also be performed by external groups
– Aformalprocessisgenerallyhighlydesirablewhendealing with external groups and may extend any of the above approaches
The University of 25
What is an informal review?
The University of 26
Informal reviews
– Reviews may be far less formal – Pair-programming
– “Overtheshoulder”
– Walkthroughs
– Presentations
– Self-guidedreviews – Checklists
– Ongoing informal reviews may easily be as thorough as a formal review, overall
The University of 27
Informal reviews
– Formal reviews are far more effective than informal reviews
– Informal reviews are far more convenient than formal reviews
The University of 28
How about a semi-formal code review?
The University of 29
Purpose
– Have a clear purpose for the review
– Identify all relevant project specifications
– Identify how the specifications can be verified
– Identify who the relevant stakeholders are
The University of 30
Change List/Pull Request reviews
– A complete, ‘working’ project exists
– A set of changes are to be applied
– The changes must be reviewed before they are accepted
The University of 31
Change List/Pull Request reviews
The University of 32
Change List/Pull Request reviews
The University of 33
Change List/Pull Request reviewers
– Owner
– Isthischangewanted?
– Technicalreviewer
– Does this change work? (specialised knowledge)
– Technicalreviewer
– Isthischangemaintainable?(generalknowledge)
• (Can I understand it?)
The University of 34
A brief visit to Design by Contract (DbC)
– A software design approach for program correctness
– Also known as contract programming, programming by
contract, design-by-contract programming
– Definition of formal, precise and verifiable interface specification for software components
– Pre-conditions,postconditionsandinvariants(contract)
The University of 35
Reviewing DbC change lists
– Definition of formal, precise and verifiable interface specification for software components
– Pre-conditions,postconditionsandinvariants(contract)
– Are the pre-conditions met?
– Are the post-conditions met?
– Are the invariants invariant?
– Are these likely to stay so?
– How hard is it to verify?
The University of 36
Reviewing DbC change lists
– How do the changes relate to the contract? – Documentationofthechange
– How do you verify this? – Reviewerselection
– Documentationofproject – Projectprocesses
• testing
The University of 37
Change List/Pull Request reviews
– Owner
– Does the documentation agree with the specifications?
– Technicalreviewer
– Aretheretests?
– Dothetestsverifythepre/postconditionsaremet?
– Technicalreviewer
– Isthecodewrittenaccordingtothestyleguide? – Doesthecodeuseappropriatedesignpatterns?
The University of 38
Change List review challenges
– Size
– The more there is to read, the more scope there is to miss
problems
– Manysmallcodereviewsaremoremanageable(somewhat like unit tests vs blackbox system tests)
– A large code review can take so much time that special planning is needed
– May need to place limits on acceptable sizes for review
The University of 39
Change List review challenges
– Scope
– Changesthataffectmanyprocessesmayneedmore
reviewers for the required technical knowledge
– May sometimes be helped by multiple, smaller changelists
– Maybringspecificationsfromdifferentprocessesintoconflict, requiring management review
• May also be triggered by small changes with big impacts, such as changing JDK version
The University of 40
Change List review challenges
– Complexity
– Fast,efficient,codemaybehardtoreadandhardto
understand
– Isthecodecomplexity,thusreviewcomplexityand maintenance complexity, worth the improvement?
– Doesthecomplexityaffectmaintainabilityandtestability?
The University of 41
Change List review challenges
– Confusion
– What is the change meant to be for?
– May be doing too many unrelated things
– May be making unnecessary changes
– May even be about a disagreement between colleagues (my approach is better!)
The University of 42
Change List review challenges
The University of 43
Change List review challenges
The University of 44
Change List review challenges
– ‘Fixsound’
The University of 45
–
Changes look reasonable
– Should it be removed completely?
– Who decides?
Change List review challenges
– ‘Fixsound’
– Changesmaybegood changes, but they’re not related to sound any more.
The University of 46
Change List review techniques
– Formalise the review process
– Allchangesmustbereviewed
– Wealreadyhavereviewroles–Owner,Readability,Technical
– Theplanningandpreparationareonlyneededonceper project (plus for each major change in direction)
– Theprojectneedsclearspecifications,requirements,and sufficient documentation
– TheCLmustmeettherequirements
– Changesarerecommendedandeitheractedonordisagreed
with and the result re-evaluated
The University of 47
Change List review comments
– Beclear
– Beobjective
– Statetheissue
– State what is needed to fix it
– E.g., “Document this method”
– E.g., “These braces aren’t needed, please remove”
– E.g., “Resubmit this PR as two separate PRs”
– E.g., “Use informative variable names”
The University of 48
Change List review examples
– This needs tests
– This needs unit tests
– This needs integration tests
– This needs documentation
– What is this for?
– This doesn’t follow the style guide
– Resubmit without the temp files
The University of 49
Change List review techniques
– Automation is great! – Whenitworks
– Codestyle
– Coverage
– Test suites
– Performancetests
– Spelling (A little more challenging in code)
– Common code issues
The University of 50
Reviewing larger projects
– You may need to review complete, larger projects, rather than small changes.
– A formal review process will ensure all parties understand what is expected
– The code inspection will be similar to a changelist review, but with no existing base to compare to
– More work is needed to verify requirements
The University of 51
Change List review comments
– E.g., “Document this method”
– E.g., “These braces aren’t needed, please remove”
– E.g., “Split this file into one per class, for readability”
– E.g., “Use informative variable names”
– A report on a set of changes would discuss why the comments were made, the benefits, the problems, and discuss other approaches
– A code review is far more concise and directed
The University of 52
References
– Fagan, M. E. (1976). “Design and code inspections to reduce errors in program development”. IBM Systems Journal. 15 (3): 182–211. doi:10.1147/sj.153.0182
– IEEEStd.1028-1997,”IEEEStandardforSoftware Reviews“, doi:10.1109/IEEESTD.2008.4601584
The University of 53