This project is read-only.

Yet again about coding style and xml comments

Dec 28, 2010 at 1:48 AM
Edited Dec 28, 2010 at 2:03 AM

It’s been about a month since I updated my private branch of Sharpbox from the latest sources from the trunk. This morning I fetched the latest version and tried to compile it.

It was quite unpleasant surprise to see bunch of warnings about missing XML comments or “Parameter 'XXX' has no matching param tag in the XML comment
When Dirk was the only developer on the project this wasn’t the case. The project was compiling without any warnings at all.

So if you write a XML comment please make sure it includes all the method’s arguments and doesn’t cause any warnings during compilation.


We use C# for this project, aren’t? So why some of the methods have signatures similar to the following sample? This sample is actually uses two styles for the arguments naming convention.

private void FileStreamCopyCallback(long ReadByteTotal, long TotalLength, params Object[] data)

It should be changed to:

private void FileStreamCopyCallback(long readByteTotal, long totalLength, params object[] data)

It might be a good idea to read .NET Framework 4 Guidelines for Names

Dec 28, 2010 at 2:01 AM
Edited Dec 28, 2010 at 12:26 PM

Couple more rants:

  1. In the class OAuthServiceManager the event names must be capitalized
  2. public delegate void WebRequestExecuted(Object sender, WebRequest request, TimeSpan timeNeeded, HttpStatusCode statusCode, Stream resultStream);
    Again this goes against C# and .Net coding standards. A event must have the following signature  EventName(object sender, XxxxEventArgs e)
    Where XxxxEventArgs contains all the arguments that you want to pass to the event handler.
Dec 28, 2010 at 2:46 AM
List<string> items = serializer.ReadObject(tokenStream) as List<string>;
// build the token
return LoadToken(items);
What is the point of using AS if the returned value is never checked for null?!
If you know for sure the object type, then use simple casting to type. Otherwise please do check the return value.
Dec 28, 2010 at 12:07 PM
Hi Yury,
you are right, in the last weeks a used not so much time into coding conventions. Just a second ago I checked in a couple of changes which eliminates the warnings, moved the event arguments into own classes derived from EventArgs and added the typecheck in the account token serializer.
In future I will have a closer look into this.
Thanks and have a good new years eve
Dirk
2010/12/28 scode <notifications@codeplex.com>

From: scode

List<string> items = serializer.ReadObject(tokenStream) as List<string>;
// build the token
return LoadToken(items);
What is the point of using AS if the returned value is never checked for null?!
If you know for sure the object type, then use simple casting to type. Otherwise please do check the return value.

Read the full discussion online.

To add a post to this discussion, reply to this email (sharpbox@discussions.codeplex.com)

To start a new discussion for this project, email sharpbox@discussions.codeplex.com

You are receiving this email because you subscribed to this discussion on CodePlex. You can unsubscribe or change your settings on codePlex.com.

Please note: Images and attachments will be removed from emails. Any posts to this discussion will also be available online at codeplex.com