What do you look for in code review?

[olduncleamos]

On the coding practice side proper exception handling/logging will be
on the top of my list, followed by (in no specific order) null
checking, use of enumeration instead of return code, functions that do
way too much, ....

On the ASP.NET side, I look for user control that goes off and does
everything without communicating with the hosting page, misuse of
viewstate, not turning off ValidateRequest, ....


What kind of things do you look for when you conduct code review for
an ASP.NET project?

Any input/suggestion will be greatly appreciated.

 

[Jon Skeet [C# MVP]]

On the coding practice side proper exception handling/logging will be
on the top of my list, followed by (in no specific order) null
checking, use of enumeration instead of return code, functions that do
way too much, ....

On the ASP.NET side, I look for user control that goes off and does
everything without communicating with the hosting page, misuse of
viewstate, not turning off ValidateRequest, ....


What kind of things do you look for when you conduct code review for
an ASP.NET project?

Any input/suggestion will be greatly appreciated.

1) Can you prove it works (with unit and/or acceptance tests)?
2) Does it work as simply as it can?

Anything else pretty much rests on these two

 

[Ken Foskey]

What kind of things do you look for when you conduct code review for an
ASP.NET project?

Look at Gilb on Code Inspection. Inspection and review are two
different things. There are many ideas around inspection that will llow
you to do a much better job.

Ta
Ken

 

[Arne Vajhøj]

On the coding practice side proper exception handling/logging will be
on the top of my list, followed by (in no specific order) null
checking, use of enumeration instead of return code, functions that do
way too much, ....

On the ASP.NET side, I look for user control that goes off and does
everything without communicating with the hosting page, misuse of
viewstate, not turning off ValidateRequest, ....

What kind of things do you look for when you conduct code review for
an ASP.NET project?

Any input/suggestion will be greatly appreciated.

Not ASP.NET specific but ...

Fitness:
- does the code implement the previously agreed and reviewed design
- is the code following normal layering and structuring
- does the code meet support the scalability model in the design
- are the costly operations done in an optimal way
- does the code have proper exception handling

Code style:
- does the code have proper exception handling
- are there any dead or otherwise unnecessary code
- is the code following coding convention

Security:
- data validation
- use of parameters with database
- use of POST for AJAX calls

Arne

 

[olduncleamos]

Not ASP.NET specific but ...

Fitness:
- does the code implement the previously agreed and reviewed design
- is the code following normal layering and structuring
- does the code meet support the scalability model in the design
- are the costly operations done in an optimal way
- does the code have proper exception handling

Code style:
- does the code have proper exception handling
- are there any dead or otherwise unnecessary code
- is the code following coding convention

Security:
- data validation
- use of parameters with database
- use of POST for AJAX calls

Arne

Thank you all for your great input. It is appreciated.