This project is read-only.

Unified coding style - Refactoring code

Jun 14, 2011 at 10:54 AM

Hello guys!

I would like to put this on the table, to make the code more maintainable and readable in the future. I would recomend we use some sort of tool/or manual work for refactoring.

For example, I am using ReSharper from Jetbrains which is a really good tool! Anyways, here's what I suggest.

1. Remove explicit declarations. This should never really be needed, when it is neaded, think about renaming the method or wrapping the operation in a method that is understandable instead.

Example:

IEnumerable<Dictionary<int, string>> myListOfDictionaries = new IEnumerable<Dictioary<int, string>>();

This could be re-written as:

var myListOfDictionaries = new IEnumerable<Dictionary<int, string>>();

There is no need to say that it is a IEnumerable<Dictionary<int, string>> because we already know that.

2. Use object initializers.

Example:

Forum f = new Forum();
f.Name = dr.GetString("ForumName");
f.Description = dr.GetString("ForumDescription");
f.ShortName = dr.GetString("ForumShortName");
f.Id = dr.Get<int>("ForumId");
f.TopicCount = dr.Get<int>("ForumTopicCount");
f.MessageCount = dr.Get<int>("ForumMessageCount");

This could be re-written with object initializers to the following:

var forum = new Forum
			{
				Name = dr.GetString("ForumName"),
			        Description = dr.GetString("ForumDescription"),
			        ShortName = dr.GetString("ForumShortName"),
			        Id = dr.Get<int>("ForumId"),
			        TopicCount = dr.Get<int>("ForumTopicCount"),
			        MessageCount = dr.Get<int>("ForumMessageCount")
			};
I also changed the variable name because "f" doens't make sense if you want to use it in a later case and you forgot what the type was.

3. Using this is ambigious, when you are using member variables inside your context and the variable name is unique, you do not need to use that keyword.

4. Remove unecessary code.

Example:

if (number == null)
{
	return shortName;
}
else
{
	number++;
	return searchShortName + number.Value.ToString();
}

Here it is not necessary to have the else block so this could be re-written as:

if (number == null)
{
	return shortName;
}

return searchShortName + (number+1).Value.ToString();

These are a few of the things that I've noticed, if it is okay with you guys I'd like to start refactoring bits and piences, as a project grows it's really neat to have everything easy to understand.

// Filip


Jun 14, 2011 at 2:40 PM

Personally I really like this idea.  ReSharper does a great job of detecting this and making very clean automatic changes.  I have recently become a big proponent of 'var', but only because I have hover-intellisense that shows me the originally type in Visual Studio.  If there are those developing outside of VS, they may want to weigh in on that one.  However, since I think this is primarily developed in VS with a VS solution file, I'd personally thumbs-up each of the four changes you listed here.  My current fork of the project that I'm working to make palatable for reintegration has many of the 2, 3, and 4 enhancements throughout.

Jun 14, 2011 at 4:19 PM

Hi All,

The only problem I see is that as an add-in it does not work with Express edition of Visual Studio (Web Dev Express), so maybe this would not encourage new team members... (Its hard to develop an os project with a closed enviroment)

What do you think?

 

If we dismiss the problem from above, I think using ReSharper would be a good idea, we should go for it. If we all agree on using it, we should ask for free Licenses of ReSharper for open source projects.

Afterwards, we can all set the rules for code analysis (default should be good) and someone (you if you like) could do a "complete" refactoring and code analysis based on those rules into the trunk.

Kind Regards,

Jorge

Jun 14, 2011 at 8:27 PM
Edited Jun 15, 2011 at 5:54 AM

Hi All,

Jorge you do not  need resharper to refactor your code, it just makes it a lot easier, the thing is, once we've refactored bits and pieces, new contributors can just adapt to the standard in the code. And if code is committed that needs refactoring, one of us that has ReSharper can just easily do that job. Or the person can refactor manually!

What do you think about that?

I have a VS2010 Premium version that I use here so I can't see those "problems" that you spotted out. Good catch! But as I mentioned, I don't think we should require ReShaper on the developers IDE. It's just easier to conform by the code-rules if you do have it.

 

Best regards,

Filip

Jun 15, 2011 at 10:31 AM

Hi,

You are right! its not necessary...

It could be a good solution to use ReSharper if the contributor has a non express VS edition and if not, just keep the conventions in mind, in order to unify code styling whenever possible.

 

Anyway, we should not lose focus and understand that a unified code style is just one of many necessary ways to keep code readable and easy to maintain in with the goal to produce better software.

I see following steps here:

  • Start specifying the code style conventions. Until now, the only code style guide we use is the detailed in the dev guide, the Ms All-In-One Code Framework Coding Guideline (.NET part). The 4 changes you specify are a good addition!
  • Ask for ReSharper licenses to JetBrains, for current members of nearforums (and ask for future contributors!)
  • Actively use this conventions and check that this conventions are beeing used (notify if they aren't)
  • Refactor old code for code styling.

 

Keep in mind that it is better to review and notify the person who commited than editing code just for the sake of styles (beware that can be interpreted as passive agressive).

Kind Regards,

Jorge

Jun 19, 2011 at 2:03 PM

Hi,

 

Ok! So how would you like me to proceed? Maybe I could take a chunk of files in a certain area, refactor them and show the result of it? I don't have any code myself contributed yet that I can refactor, so I am better of refactoring someone elses. :)

 

Best regards,

Filip

Jun 20, 2011 at 5:54 PM

I would prefer that you start with a new feature to apply the new coding conventions in your contributions and then changing existing code.

Kind Regards,

Jorge

Jun 21, 2011 at 3:58 PM

Sounds good!

 

Best regards,

Filip