1
\$\begingroup\$

I am automating a webform, and this is what I've coded so far. I am curious if you find the code good and readable. But most important to me is to get some feedback for learning purposes.

The remove account page:

public class RemoveAccountsPage extends Page {
 private Boolean accountsToRemove() {
 by = By.className("zero-results-filter");
 return ((elementExists(by)) ? true : false);
 }
 private void selectAllAccounts() {
 by = By.id("accounts-select-all");
 element = waitForPresenceOfElement(by);
 element.click();
 }
 private void clickRemoveButton() {
 by = By.id("account-delete");
 element = waitForPresenceOfElement(by);
 element.click();
 }
 private void confirmRemovingAccounts() {
 by = By.id("confirm");
 element = waitForPresenceOfElement(by);
 element.click();
 //driver.findElement(By.id("NO_DEAL_VIA_MP")).click()
 }
 public void removeAccounts() throws Exception {
 String request = Page.URL + "/hu/accounts/index.html";
 goToWebPage(request);
 Boolean result = accountsToRemove();
 if(result == false) {
 selectAllAccounts();
 clickRemoveButton();
 confirmRemovingAccounts();
 }
 Thread.sleep(1000);
 }
}

Main class:

public class AccountManager {
 public static void main(String args[]) throws Exception {
 Bot bot = new Bot();
 bot.setUp();
 ........
 RemoveAccountsPage removeAccountsPage = new 
 RemoveAccountsPage();
 removeAccountsPage.removeAccounts();
 ........
 bot.tearDown();
 } 
}
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jul 26, 2017 at 13:46
\$\endgroup\$

2 Answers 2

1
\$\begingroup\$

Some lines may be simplified.

1) elementExists() already returns boolean, so use it directly:

return ((elementExists(by)) ? true : false);
to
return elementExists(by);

2) You may inline a method calls into condition statements:

Boolean result = accountsToRemove();
if(result == false) {
to 
if(!accountsToRemove()) {

3) Name the main test method correspondingly:

removeAccounts()
to
testRemovingAllAccounts()
userRemovesAllAccounts()
answered Jul 26, 2017 at 22:58
\$\endgroup\$
0
\$\begingroup\$

It would be nice to see your implementation of Page.

I find the points made by Dmytro very valid and want to add a couple of things more.

Regarding your accountsToRemove() method; somehow this method doesn ́t have a name that tells me exactly what it does. I ́d expect to give a list of accounts that should be removed, but instead tells me whether an account can be removed right?

On the same note your waitForPresenceOfElement(by) returns an element located by 'by', however I ́d expect for it just to wait.

I assume that your by and element variables are global in your Page class, in which case you don ́t have to pass them as parameters. However I don ́t like global variables, and so I ́d tell you to remove them and keep the parameters =)

Since you already have a base Page where you find your elements etc, I ́d go one step further and have another method clickElement(...) instead of calling element.click(). This helps you in case the element is not clickable or with StaleReferenceElementException (pretty common I ́d say)

answered Aug 3, 2017 at 10: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.