You wont find a single project where long method code smell is not present. I find often this code smell often in my projects, and developers do not deliberately introduce long methods, it just got introduced along the way when we introduce features.
What happen when you find a long method?
- You wonder why is this method is so long.
- You try to understand what the code is doing.
- You start hate the software because its full of lines after line but no end of the method.
- You lost the track where you ware before.
- And so on.
You can list hundreds of line like the above listing about long method, the story will go on and on. here is what “Wikipedia” has to say about the long method,
“Long method: a method, function, or procedure that has grown too large.”
Bellow an example of a long method is given
void SolutionLinkBrowserNavigating(object sender, WebBrowserNavigatingEventArgs e)
{
var pageName = e.Url.ToString().Split('/').LastOrDefault();
if (IsLinkToTmeTool(pageName))
{
if (pageName != null)
{
var toolName = Regex.Replace(pageName, TMEToolPrefixPattern, "", RegexOptions.IgnoreCase);
e.Cancel = true;
if (PackageLoader.Instance.IsToolAvaiableForCurrentUser(toolName))
{
ToolManager.Value.ActivateToolCommand.Execute(toolName);
}
else
{
new Lazy<IMessageBoxPresenter>().Value.Show(
StringTable.ToolNotAvailableErrorMessageText, StringTable.ToolNotAvailableErrorMessageTitle, DialogButton.OK);
return;
}
}
}
else
{
var queryString = e.Url.Query;
const string pattern = @"^\?.*POwner=(?<Owner>[^&]+)";
var match = Regex.Match(queryString, pattern, RegexOptions.IgnoreCase);
if (match.Success)
{
var ownerPackageId = match.Groups["Owner"].Value;
var allowedPackageIds = PackageLoader.Instance.GetAllowedPackageIds().ToList();
if (allowedPackageIds.Contains(ownerPackageId) == false)
{
e.Cancel = true;
new Lazy<IMessageBoxPresenter>().Value.Show( StringTable.PageNotAvailableErrorMessageText, StringTable.PageNotAvailableErrorMessageTitle, DialogButton.OK);
return;
}
}
}
}
Bellow a edited version of the code is given. where the code is refactored with extract method and looks more understandable, even though you can refactor the code to a much simpler version, but this is given there as an example.
void SolutionLinkBrowserNavigating(object sender, WebBrowserNavigatingEventArgs e)
{
var pageName = e.Url.ToString().Split('/').LastOrDefault();
if (IsLinkToTmeTool(pageName))
LoadLinkedTool(e, pageName);
else
HandleOtherLink(e);
}
private static void HandleOtherLink(WebBrowserNavigatingEventArgs e)
{
var queryString = e.Url.Query;
const string pattern = @"^\?.*POwner=(?<Owner>[^&]+)";
var match = Regex.Match(queryString, pattern, RegexOptions.IgnoreCase);
if (match.Success == false)
return;
var ownerPackageId = match.Groups["Owner"].Value;
var allowedPackageIds = PackageLoader.Instance.GetAllowedPackageIds().ToList();
if (allowedPackageIds.Contains(ownerPackageId) == false)
{
e.Cancel = true;
DisplayMessage(StringTable.PageNotAvailableErrorMessageText, StringTable.PageNotAvailableErrorMessageTitle);
}
}
private static void DisplayMessage(string message, string title)
{
var messageBoxPresenter = new Lazy<IMessageBoxPresenter>();
messageBoxPresenter.Value.Show(message,title, DialogButton.OK);
}
private void LoadLinkedTool(WebBrowserNavigatingEventArgs e, string pageName)
{
if (pageName != null)
{
var toolName = Regex.Replace(pageName, TMEToolPrefixPattern, "", RegexOptions.IgnoreCase);
e.Cancel = true;
if (PackageLoader.Instance.IsToolAvaiableForCurrentUser(toolName))
ToolManager.Value.ActivateToolCommand.Execute(toolName);
else
DisplayMessage(StringTable.ToolNotAvailableErrorMessageText,StringTable.ToolNotAvailableErrorMessageTitle);
}
}
There are lot of refactoring tips and techniques to handle long method, and this is just the tip of the iceberg, this is just an example you can do many things and there are huge possibilities. Until next time me Md. Masudur Rahman Signning out.
