1
\$\begingroup\$

I am trying to design some classes which I intent to use as a framework. I would like them to be compliant to SOLID design principles.

I had a basic structure and then I implemented Strategy design pattern and tried adding dependency injection as well.

Would like this to be reviewed so that I know if my understanding is correct or can still be made better.

public interface IBrowser
{
 IWebDriver GetWebDriver(string webDriverFolderPath);
}
public class EdgeBrowser : IBrowser
{
 public IWebDriver GetWebDriver(string webDriverFolderPath)
 {
 var options = new EdgeOptions()
 {
 PageLoadStrategy = PageLoadStrategy.Eager,
 UseInPrivateBrowsing = true
 };
 return new EdgeDriver(webDriverFolderPath, options, TimeSpan.FromSeconds(60));
 }
}
public class IEBrowser : IBrowser
{
 public IWebDriver GetWebDriver(string webDriverFolderPath)
 {
 var options = new InternetExplorerOptions()
 {
 IntroduceInstabilityByIgnoringProtectedModeSettings = true
 };
 return new InternetExplorerDriver(webDriverFolderPath, options, TimeSpan.FromSeconds(60));
 }
}
public interface IConfiguration
{
 string GetConfiguration(string key);
}
public class AppSettingsConfiguration : IConfiguration
{
 public string GetConfiguration(string key)
 {
 return ConfigurationManager.AppSettings[key].ToString();
 }
}
public class WebDriver
{
 private static Dictionary<Enums.Browsers, IBrowser> Browsers = new Dictionary<Enums.Browsers, IBrowser>();
 private static Dictionary<string, Enums.Browsers> BrowserMapper = new Dictionary<string, Enums.Browsers>();
 private static AppSettingsConfiguration _AppSettingsConfiguration;
 static WebDriver()
 {
 Browsers.Add(Enums.Browsers.Edge, new EdgeBrowser());
 Browsers.Add(Enums.Browsers.IE, new IEBrowser());
 Browsers.Add(Enums.Browsers.Chrome, new ChromeBrowser());
 Browsers.Add(Enums.Browsers.Firefox, new FirefoxBrowser());
 BrowserMapper.Add("Edge", Enums.Browsers.Edge);
 BrowserMapper.Add("IE", Enums.Browsers.IE);
 BrowserMapper.Add("Internet Explorer", Enums.Browsers.IE);
 BrowserMapper.Add("Chrome", Enums.Browsers.Chrome);
 BrowserMapper.Add("Firefox", Enums.Browsers.Firefox);
 BrowserMapper.Add("edge", Enums.Browsers.Edge);
 BrowserMapper.Add("ie", Enums.Browsers.IE);
 BrowserMapper.Add("internet explorer", Enums.Browsers.IE);
 BrowserMapper.Add("chrome", Enums.Browsers.Chrome);
 BrowserMapper.Add("firefox", Enums.Browsers.Firefox);
 BrowserMapper.Add("internetexplorer", Enums.Browsers.IE);
 BrowserMapper.Add("InternetExplorer", Enums.Browsers.IE);
 }
 public WebDriver(AppSettingsConfiguration appSettingsConfiguration)
 {
 _AppSettingsConfiguration = appSettingsConfiguration;
 }
 public IWebDriver GetWebDriver(string browser)
 {
 var browserName = BrowserMapper[browser];
 return Browsers[browserName].GetWebDriver(_AppSettingsConfiguration.GetConfiguration("WebDriverFolderPath"));
 }
}
public void Initialize()
{
 if (WebDriver == null)
 {
 WebDriver = new WebDriver(new AppSettingsConfiguration()).GetWebDriver(BaseData.Browser);
 }
}
200_success
145k22 gold badges190 silver badges478 bronze badges
asked Jan 17, 2019 at 11:15
\$\endgroup\$
2
  • \$\begingroup\$ Perhaps GetWebDriver() could just use the Browsers enumeration to avoid having to create duplicate mappings like "internetexplorer", "InternetExplorer", and "internet explorer". \$\endgroup\$ Commented Jan 17, 2019 at 14:22
  • \$\begingroup\$ @Shelby115 thanks for the reply. Initialize method receives data read from an excel. So browser name can vary and I was not able to think about any other way to map user input to enums... \$\endgroup\$ Commented Jan 17, 2019 at 14:50

1 Answer 1

2
\$\begingroup\$

I'm not really sure that there is any good argument having these static fields, especially when it comes to unit testing the WebDriver class.

public class WebDriver
{
 private static Dictionary... // this
 private static Dictionary... // this
 private static AppSetting... // this
 static WebDriver() // this
 {
 ...
 }
 ...
}

I would suggest you change those to private readonly fields instead.

and then we have this Initialize() method hanging out on it's own -- which means your code could not compile as is.

public class Foo // where is the class?
{
 public void Initialize()
 {
 if (WebDriver == null)
 {
 WebDriver = new WebDriver(new AppSettingsConfiguration())
 .GetWebDriver(BaseData.Browser);
 }
 }
}

I highly would not recommend passing around the AppSettingsConfiguration class, when it seems like the only time you use it is to pull a string value by calling _AppSettingsConfiguration.GetConfiguration("WebDriverFolderPath").

It would make more sense for you to handle reading your configuration somewhere else and not make the WebDriver responsible for handling that... (This actually breaks your "S" in the SOLID principle).

I would probably advise to consider refactoring it along these lines...

public enum Browsers { Unknown, Ie, Chrome, Edge, FireFox }
public class WebDriverFactory
{
 private readonly string _path;
 public WebDriverFactory(string path)
 {
 if (String.IsNullOrEmpty(path)) throw new ArgumentNullException(nameof(path));
 // also, consider to a check that the path exists
 _path = path;
 }
 public IWebDriver Construct(Browsers browser)
 {
 switch (browser)
 {
 case Browser.Edge:
 {
 var options = new EdgeOptions()
 {
 PageLoadStrategy = PageLoadStrategy.Eager,
 UseInPrivateBrowsing = true
 };
 return new EdgeDriver(_path, options, TimeSpan.FromSeconds(60));
 }
 case.Unknown:
 throw new Exception("not a valid browser");
 default:
 throw new NotImplementedException();
 }
 }
}
answered Jan 17, 2019 at 16:06
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.