patterncsharpMinor
Are private extension methods a bad thing to use?
Viewed 0 times
arebadprivateextensionmethodsusething
Problem
In C# I have the following two extension methods.
But since the class is static, I can write a private extension method to make the syntax more readable like this.
Is there any inherently bad code smell about private extension methods? Which would is better to use? The only issue I see is that if the code is later refactored into a class the methods will have to be rewritten.
public static void WaitForMilliseconds(this IWebDriver driver, int milliseconds)
{
var timeout = new TimeSpan(0, 0, 0, 0, milliseconds);
WaitForTimeout(driver, timeout);
}
public static void WaitForSeconds(this IWebDriver driver, int seconds)
{
var timeout = new TimeSpan(0, 0, 0, seconds);
WaitForTimeout(driver, timeout);
}
private static void WaitForTimeout(IWebDriver driver, TimeSpan timeout)
{
try
{
new WebDriverWait(driver, timeout).Until(x => false);
}
catch (Exception){}
}But since the class is static, I can write a private extension method to make the syntax more readable like this.
public static void WaitForMilliseconds(this IWebDriver driver, int milliseconds)
{
var timeout = new TimeSpan(0, 0, 0, 0, milliseconds);
driver.WaitForTimeout(timeout);
}
public static void WaitForSeconds(this IWebDriver driver, int seconds)
{
var timeout = new TimeSpan(0, 0, 0, seconds);
driver.WaitForTimeout(timeout);
}
private static void WaitForTimeout(this IWebDriver driver, TimeSpan timeout)
{
try
{
new WebDriverWait(driver, timeout).Until(x => false);
}
catch (Exception){}
}Is there any inherently bad code smell about private extension methods? Which would is better to use? The only issue I see is that if the code is later refactored into a class the methods will have to be rewritten.
Solution
In my opinion, extension methods are only a (pretty nice) syntactic sugar for a static helper method. Under the hood, the IL translates your extension method call to a call on the static method, so really it's just an other way to write methods.
The extension method do not have a private access to the type they are extending either, so changing a static method into an extension one will not break any encapsulation.
As for the private keyword, it is indeed the correct accessibility level since your method will not be used outside of your class.
According to MSDN's Guidelines, the major problem of extension methods is the following :
When using an extension method to extend a type whose source code you cannot change, you run the risk that a change in the implementation of the type will cause your extension method to break.
Also, if someday IWebDriver defines a new method called "WaitForTimeout", your extension method won't be called anymore... But the risk is really pretty low.
So if you think readability is increased by using that extension method, use it - you won't violate any principles or break anything, see it as a nice syntactic sugar.
The extension method do not have a private access to the type they are extending either, so changing a static method into an extension one will not break any encapsulation.
As for the private keyword, it is indeed the correct accessibility level since your method will not be used outside of your class.
According to MSDN's Guidelines, the major problem of extension methods is the following :
When using an extension method to extend a type whose source code you cannot change, you run the risk that a change in the implementation of the type will cause your extension method to break.
Also, if someday IWebDriver defines a new method called "WaitForTimeout", your extension method won't be called anymore... But the risk is really pretty low.
So if you think readability is increased by using that extension method, use it - you won't violate any principles or break anything, see it as a nice syntactic sugar.
Context
StackExchange Code Review Q#30118, answer score: 7
Revisions (0)
No revisions yet.