0
\$\begingroup\$

I try to improve my code so that I don't have to program a try - catch block all the time.

e.g. at the moment I have to write many codeblocks like this:

try {
    $login_feld = $SeleniumObj->driver->findElement(WebDriverBy::id("login_feld"));
} catch(NoSuchElementException $exception) {
    $SeleniumObj->setErrorMessage("1 ID login_feld does not exist.", $exception,  true, "login_field");
    break;
}
try {
    $password_field = $SeleniumObj->driver->findElement(WebDriverBy::id("password_field"));
} catch(NoSuchElementException $exception) {
    $SeleniumObj->setErrorMessage("1 ID password_field does not exist.", $exception,  true, "login_field");
    break;
}
try {
    $field3 = $SeleniumObj->driver->findElement(WebDriverBy::id("field3"));
} catch(NoSuchElementException $exception) {
    $SeleniumObj->setErrorMessage("1 ID field3 does not exist.", $exception,  true, "login_field");
    break;
}

My goal is to reduce this to one line.

Therefore I outsourced the whole driver->findElement(WebDriverBy::id("xy")); to a new function from the class Selenium called ById

/**
 * 
 * @param id $id
 * @param bool $takeScreenshot
 * @return void
 */
public function ById($id, $takeScreenshot=false)
{
    try {
        return $this->driver->findElement(WebDriverBy::id($id));

    } catch(NoSuchElementException $exception) {

        $this->setErrorMessage("ID '$id' does not exist.", $exception,  $takeScreenshot, $id);
        //break;  <- does not work in this context
    }
}

Now theoretically I can code the whole block like this:

$login_feld = $SeleniumObj->ById('login_feld', true);
$password_field = $SeleniumObj->ById('password_field', true);
$field3 = $SeleniumObj->ById('field3', true);

However, one problem still persist. I can't call break in the exception block in the function ById, otherwise I get

break' not in the 'loop' or 'switch' context


Now to solve this I could instead return a status, e.g. false if the code reached in the exception block and then check this before proceeding

/**
 * 
 * @param id $id
 * @param bool $takeScreenshot
 * @return void
 */
public function ById($id, $takeScreenshot=false)
{
    try {
        return $this->driver->findElement(WebDriverBy::id($id));

    } catch(NoSuchElementException $exception) {

        $this->setErrorMessage("ID '$id' does not exist.", $exception,  $takeScreenshot, $id);
        return false;
    }
}

-

$login_field = $SeleniumObj->ById('login_field', true);
if ($login_field == false) { break; }
$pw_field = $SeleniumObj->ById('pw_field', true);
if ($pw_field == false) { break; }
$field3 = $SeleniumObj->ById('field3', true);
if ($field3 == false) { break; }

But now I need two lines per block instead of one. Is it possible to improve this even further?

\$\endgroup\$
2
  • \$\begingroup\$ 1) Do not edit the code in your question after receiving answers. Doing so goes against the nature of Q&A sites. 2) Your code looks very hypothetical. Is this actual code in use? Please take a look at the help center. \$\endgroup\$
    – Mast
    Commented Mar 21, 2017 at 14:53
  • \$\begingroup\$ @Mast, I had to edit the code to clarify it. I just showed that the try-catch blocks are actually in a switch statement. And yes this is real code. \$\endgroup\$
    – Black
    Commented Mar 21, 2017 at 14:56

2 Answers 2

1
\$\begingroup\$

I don't understand what you are intending to do with break in your catch blocks to begin with. Even in your first code example, you are not within a loop construct to where this makes sense.

In your new code, you should have method name begin with lower case (i.e. byId) as this is pretty much standard in PHP. I don't really see what value this method is bringing though, as I would tend to agree with the post by @YourCommonSense that you might simplify your operation by using a looping construct. However I might suggest you put the try-catch within the loop if your intent is to be able to engage in the try-catch for each element rather than for the collection of elements as a whole.

For example:

$elements = [
    'login_feld' => null,
    'password_field' => null,
    'field3' => null
];
foreach($elements as $id => $notUsed) {
    try {
        $elements[$id] = $SeleniumObj->driver->findElement(WebDriverBy::id($id));
    } catch (NoSuchElementException $e) {
        $SeleniumObj->setErrorMessage(
            '1 ID ' . $id . 'does not exist.',
            $e,
            true,
            $id
        );
        // if you want to break here, you now have proper loop context
        // break;
    }
}

// if successful, you can now access the elements from $elements array

This approach is scalable as you add more elements into your test cases, as you could just add new id keys into the element array and not worry about polluting your global namespace with a bunch of $[elementId] variables. You also now have the ability to simply look at the value stored at each key to determine if the element was found. So for example, if you are going to perform a test action against a certain element, you can test for it like:

if(!is_null($elements['login_field'])) {
    // execute tests against element
} else {
    // log test fail
}

This approach might totally alleviate your need to break altogether, meaning you could now continue to execute your test suite even if one of the elements you expect to be present is missing. You could just fail those particular tests relevant to the missing element.

\$\endgroup\$
0
-3
\$\begingroup\$

Every time you see repeated lines of code, they can be reduced to a single line run in a loop, so you can do:

try {
    foreach(["login_field","password_field","field3"] as $field)
    {
        $$field = $SeleniumObj->driver->findElement(WebDriverBy::id($field));
    }
} catch(NoSuchElementException $exception) {
    $SeleniumObj->setErrorMessage("1 ID $field does not exist.", $exception, true, $field);
    break;
}
\$\endgroup\$
0

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.