HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptModerate

Functions to hide and reclaim first visible publication on a page using Selenium

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
visiblereclaimseleniumfirstpublicationpageusingandfunctionshide

Problem

I have the following functions in JavaScript:

function SelectFirstVisiblePublicationAndHideIt ( PageObj )
{
    var hideButtonXpath = "//img[@title='To hide this item in your profile, mark it as invisible']";
    var hideButtonArray;
    var numberOfHideButton;
    var publicationTitle;

    aqUtils.Delay(500);
    hideButtonArray = PageObj.EvaluateXpath(hideButtonXpath);
    numberOfHideButton = hideButtonArray.length;
    hideButtonArray[0].Click();
    publicationTitle = hideButtonArray[0].parent.parent.FindChildByXPath("//a").innerText;

    return publicationTitle;
}

function SelectFirstVisiblePublicationAndReclaimIt ( PageObj )
{
    var reclaimButtonXpath = "//button[@title='Claim this publication and stay on this page']";
    var reclaimButtonArray;
    var numberOfReclaimButton;
    var publicationTitle;

    aqUtils.Delay(500);
    reclaimButtonArray = PageObj.EvaluateXpath(reclaimButtonXpath);
    numberOfReclaimButton = reclaimButtonArray.length;
    reclaimButtonArray[0].Click();
    publicationTitle = reclaimButtonArray[0].parent.parent.FindChildByXPath("//a").innerText;

    return publicationTitle;
}


SelectFirstVisiblePublicationAndHideIt and SelectFirstVisiblePublicationAndReclaimIt are very similar except for the Xpath expression.

I am wondering if I should merge them into one function by:

  • re-naming variables and functions names in a more generic way



  • introduce an additional input argument that serves as a flag to indicate which Xpath expression is to be used



Doing so has pros and cons:

  • Pros: reduce the number of function by one



  • Cons: purpose of a function can no longer be read from its name; having one more input argument; become further away from the principle of "one function should do one thing and one thing only"



Any suggestions?

Solution

As the code is similar, they can be merged together.

I'll suggest you to break down the functions into smaller reusable chunks which will follow the Single Responsibility Principle (SRP). These functions will be testable and reusable.

Both functions are

  • Adding a delay of 500 milliseconds



  • Selecting elements from provided page



  • Clicking the first element from the collection



  • Finding an element from its ancestor and returning the innerText of it.



As these are the steps, we can similarly divide those functions into smaller functions each doing the task of one step above.

For first step (to add delay) I've not created a new function as I guess that is already created in utils module.

The first function (Step #2) will select the elements from the page using the provided xpath and return the first of them.

function getFirstVisiblePublication(page, xpath) {
    aqUtils.Delay(500);

    var elements = page.EvaluateXpath(xpath);
    return elements[0];
}


The second function combines Steps #3 & #4; as #3 only requires to click the element I haven't created new function.

The below function will accept the page in which the target element is to be searched and xpath of the element. This will call the above function to get the first element and click it.

This function will return the text/label of the anchor element which is obtained by using the first element's ancestors.

function getFirstVisiblePublicationAnchorLabel(page, xpath) {
    // Get first element
    var firstElement = getFirstVisiblePublication(page, xpath);
    firstElement.Click();

    return firstElement.parent.parent.FindChildByXPath('//a').textContent;
}


Usage:

Function can be called as follows:

var hideButtonXpath = "//img[@title='To hide this item in your profile, mark it as invisible']";
var publicationTitle = getFirstVisiblePublicationAnchorLabel(PageObj, hideButtonXpath);

Code Snippets

function getFirstVisiblePublication(page, xpath) {
    aqUtils.Delay(500);

    var elements = page.EvaluateXpath(xpath);
    return elements[0];
}
function getFirstVisiblePublicationAnchorLabel(page, xpath) {
    // Get first element
    var firstElement = getFirstVisiblePublication(page, xpath);
    firstElement.Click();

    return firstElement.parent.parent.FindChildByXPath('//a').textContent;
}
var hideButtonXpath = "//img[@title='To hide this item in your profile, mark it as invisible']";
var publicationTitle = getFirstVisiblePublicationAnchorLabel(PageObj, hideButtonXpath);

Context

StackExchange Code Review Q#144417, answer score: 15

Revisions (0)

No revisions yet.