Bug
##Bug YouYou talk about duplication of a block of code, but that code is not a duplicate in the source you posted. It's unclear if that's a bug or a posting error..
Design
##Design UseUse methods! Even short, 1-line methods will make it much easier to read and understand what's going on. In particular, duplicated code should usually be extracted for clarity and maintenance sanity. Good method names can help document the intent of method calls. If the block of code you're talking about is supposed to be identical, you can extract it into a method whose name documents what that block of code does.
General
##General Don'tDon't abbreviate. Is "dBName" supposed to be "databaseName"? Nobody unfamiliar with your code will be able to tell what it is supposed to stand for.
##Bug You talk about duplication of a block of code, but that code is not a duplicate in the source you posted. It's unclear if that's a bug or a posting error..
##Design Use methods! Even short, 1-line methods will make it much easier to read and understand what's going on. In particular, duplicated code should usually be extracted for clarity and maintenance sanity. Good method names can help document the intent of method calls. If the block of code you're talking about is supposed to be identical, you can extract it into a method whose name documents what that block of code does.
##General Don't abbreviate. Is "dBName" supposed to be "databaseName"? Nobody unfamiliar with your code will be able to tell what it is supposed to stand for.
Bug
You talk about duplication of a block of code, but that code is not a duplicate in the source you posted. It's unclear if that's a bug or a posting error..
Design
Use methods! Even short, 1-line methods will make it much easier to read and understand what's going on. In particular, duplicated code should usually be extracted for clarity and maintenance sanity. Good method names can help document the intent of method calls. If the block of code you're talking about is supposed to be identical, you can extract it into a method whose name documents what that block of code does.
General
Don't abbreviate. Is "dBName" supposed to be "databaseName"? Nobody unfamiliar with your code will be able to tell what it is supposed to stand for.
import java.util.List;
public final class Snippet {
private static final String FORM_PATH = "html/body/form[2]/";
private static final String PACE_PATH = "table[1]/tbody/tr[2]/td[2]/input[1]";
private static final String TAB_PATH = "table[1]/tbody/tr[4]/td/b";
public static void main(final String[] argv) {
getPaceNumber(null, null, null, null, 0, null);
}
private static void getPaceNumber(
final WebDriver chromeDriver,
final String dbName,
final XSSFSheet paceSheet,
final String pubName,
final int rowNumber,
final XSSFWorkbook workbook) {
System.out.println("DBID is " + dbName + " and fpn is " + pubName);
final XSSFCell cell = paceSheet.getRow(rowNumber).createCell(1);
final int defaultHeight = paceSheet.getRow(0).getHeight();
paceSheet.getRow(rowNumber).setHeight((short) (defaultHeight * 2));
if (dbName == "" || dbName.equals("null")) {
System.out.println("Null Block");
cell.setCellValue("N/A");
searchPublication(chromeDriver);
return;
}
sendAndClick(chromeDriver, dbName);
int pubPaceNumber = 0;
int dbPaceNumber;
if (hasPaceElements(chromeDriver)) {
dbPaceNumber = findPaceValue(chromeDriver);
searchPublication(chromeDriver);
sendAndClick(chromeDriver, pubName);
if (hasPaceElements(chromeDriver)) {
pubPaceNumber = findPaceValue(chromeDriver);
} else {
if (hasTabElement(chromeDriver)) {
searchPublication(chromeDriver);
sendAndClick(chromeDriver, pubName);
}
cell.setCellValue("N/A");
}
if (dbPaceNumber == pubPaceNumber) {
cell.setCellValue(dbPaceNumber);
} else {
final CellStyle style = workbook.createCellStyle();
style.setWrapText(true);
style.setAlignment(CellStyle.ALIGN_RIGHT);
cell.setCellStyle(style);
cell.setCellValue(dbPaceNumber + "\n" + pubPaceNumber);
}
} else {
if (hasTabElement(chromeDriver)) {
searchPublication(chromeDriver);
sendAndClick(chromeDriver, pubName);
if (hasPaceElements(chromeDriver)) {
cell.setCellType(Cell.CELL_TYPE_NUMERIC);
cell.setCellValue(findPaceValue(chromeDriver));
} else {
cell.setCellValue("N/A");
}
} else {
cell.setCellValue("N/A");
}
}
searchPublication(chromeDriver);
}
private static void searchPublication(final WebDriver chromeDriver) {
chromeDriver.findElement(By.xpath(".//*[@id='searchPublication']")).click();
}
private static WebElement findFormElement(final WebDriver chromeDriver, final String path) {
return chromeDriver.findElement(By.xpath(FORM_PATH + path));
}
private static List<WebElement> findFormElements(final WebDriver chromeDriver, final String path) {
return chromeDriver.findElements(By.xpath(FORM_PATH + path));
}
private static boolean hasPaceElements(final WebDriver chromeDriver) {
return !findFormElements(chromeDriver, PACE_PATH).isEmpty();
}
private static int findPaceValue(final WebDriver chromeDriver) {
final WebElement element = findFormElement(chromeDriver, PACE_PATH);
return Integer.parseInt(element.getAttribute("value"));
}
private static boolean hasTabElement(final WebDriver chromeDriver) {
return findFormElements(chromeDriver, TAB_PATH).size() == 1;
}
private static void sendAndClick(final WebDriver chromeDriver, final String key) {
findFormElement(chromeDriver, "b/b/table/tbody/tr[2]/td[2]/textarea").sendKeys("\"" + key + "\"");
findFormElement(chromeDriver, "b/b/table/tbody/tr[4]/td[2]/input[1]").click();
}
}
import java.util.List;
public final class Snippet {
private static final String FORM_PATH = "html/body/form[2]/";
private static final String PACE_PATH = "table[1]/tbody/tr[2]/td[2]/input[1]";
private static final String TAB_PATH = "table[1]/tbody/tr[4]/td/b";
public static void main(final String[] argv) {
getPaceNumber(null, null, null, null, 0, null);
}
private static void getPaceNumber(
final WebDriver chromeDriver,
final String dbName,
final XSSFSheet paceSheet,
final String pubName,
final int rowNumber,
final XSSFWorkbook workbook) {
System.out.println("DBID is " + dbName + " and fpn is " + pubName);
final XSSFCell cell = paceSheet.getRow(rowNumber).createCell(1);
final int defaultHeight = paceSheet.getRow(0).getHeight();
paceSheet.getRow(rowNumber).setHeight((short) (defaultHeight * 2));
if (dbName == "" || dbName.equals("null")) {
System.out.println("Null Block");
cell.setCellValue("N/A");
searchPublication(chromeDriver);
return;
}
sendAndClick(chromeDriver, dbName);
int pubPaceNumber = 0;
int dbPaceNumber;
if (hasPaceElements(chromeDriver)) {
dbPaceNumber = findPaceValue(chromeDriver);
searchPublication(chromeDriver);
sendAndClick(chromeDriver, pubName);
if (hasPaceElements(chromeDriver)) {
pubPaceNumber = findPaceValue(chromeDriver);
} else {
if (hasTabElement(chromeDriver)) {
searchPublication(chromeDriver);
sendAndClick(chromeDriver, pubName);
}
cell.setCellValue("N/A");
}
if (dbPaceNumber == pubPaceNumber) {
cell.setCellValue(dbPaceNumber);
} else {
final CellStyle style = workbook.createCellStyle();
style.setWrapText(true);
style.setAlignment(CellStyle.ALIGN_RIGHT);
cell.setCellStyle(style);
cell.setCellValue(dbPaceNumber + "\n" + pubPaceNumber);
}
} else {
if (hasTabElement(chromeDriver)) {
searchPublication(chromeDriver);
sendAndClick(chromeDriver, pubName);
if (hasPaceElements(chromeDriver)) {
cell.setCellType(Cell.CELL_TYPE_NUMERIC);
cell.setCellValue(findPaceValue(chromeDriver));
} else {
cell.setCellValue("N/A");
}
} else {
cell.setCellValue("N/A");
}
}
searchPublication(chromeDriver);
}
private static void searchPublication(final WebDriver chromeDriver) {
chromeDriver.findElement(By.xpath(".//*[@id='searchPublication']")).click();
}
private static WebElement findFormElement(final WebDriver chromeDriver, final String path) {
return chromeDriver.findElement(By.xpath(FORM_PATH + path));
}
private static List<WebElement> findFormElements(final WebDriver chromeDriver, final String path) {
return chromeDriver.findElements(By.xpath(FORM_PATH + path));
}
private static boolean hasPaceElements(final WebDriver chromeDriver) {
return !findFormElements(chromeDriver, PACE_PATH).isEmpty();
}
private static int findPaceValue(final WebDriver chromeDriver) {
final WebElement element = findFormElement(chromeDriver, PACE_PATH);
return Integer.parseInt(element.getAttribute("value"));
}
private static boolean hasTabElement(final WebDriver chromeDriver) {
return findFormElements(chromeDriver, TAB_PATH).size() == 1;
}
private static void sendAndClick(final WebDriver chromeDriver, final String key) {
findFormElement(chromeDriver, "b/b/table/tbody/tr[2]/td[2]/textarea").sendKeys("\"" + key + "\"");
findFormElement(chromeDriver, "b/b/table/tbody/tr[4]/td[2]/input[1]").click();
}
}
import java.util.List;
public final class Snippet {
private static final String FORM_PATH = "html/body/form[2]/";
private static final String PACE_PATH = "table[1]/tbody/tr[2]/td[2]/input[1]";
private static final String TAB_PATH = "table[1]/tbody/tr[4]/td/b";
private static void getPaceNumber(
final WebDriver chromeDriver,
final String dbName,
final XSSFSheet paceSheet,
final String pubName,
final int rowNumber,
final XSSFWorkbook workbook) {
System.out.println("DBID is " + dbName + " and fpn is " + pubName);
final XSSFCell cell = paceSheet.getRow(rowNumber).createCell(1);
final int defaultHeight = paceSheet.getRow(0).getHeight();
paceSheet.getRow(rowNumber).setHeight((short) (defaultHeight * 2));
if (dbName == "" || dbName.equals("null")) {
System.out.println("Null Block");
cell.setCellValue("N/A");
searchPublication(chromeDriver);
return;
}
sendAndClick(chromeDriver, dbName);
int pubPaceNumber = 0;
int dbPaceNumber;
if (hasPaceElements(chromeDriver)) {
dbPaceNumber = findPaceValue(chromeDriver);
searchPublication(chromeDriver);
sendAndClick(chromeDriver, pubName);
if (hasPaceElements(chromeDriver)) {
pubPaceNumber = findPaceValue(chromeDriver);
} else {
if (hasTabElement(chromeDriver)) {
searchPublication(chromeDriver);
sendAndClick(chromeDriver, pubName);
}
cell.setCellValue("N/A");
}
if (dbPaceNumber == pubPaceNumber) {
cell.setCellValue(dbPaceNumber);
} else {
final CellStyle style = workbook.createCellStyle();
style.setWrapText(true);
style.setAlignment(CellStyle.ALIGN_RIGHT);
cell.setCellStyle(style);
cell.setCellValue(dbPaceNumber + "\n" + pubPaceNumber);
}
} else {
if (hasTabElement(chromeDriver)) {
searchPublication(chromeDriver);
sendAndClick(chromeDriver, pubName);
if (hasPaceElements(chromeDriver)) {
cell.setCellType(Cell.CELL_TYPE_NUMERIC);
cell.setCellValue(findPaceValue(chromeDriver));
} else {
cell.setCellValue("N/A");
}
} else {
cell.setCellValue("N/A");
}
}
searchPublication(chromeDriver);
}
private static void searchPublication(final WebDriver chromeDriver) {
chromeDriver.findElement(By.xpath(".//*[@id='searchPublication']")).click();
}
private static WebElement findFormElement(final WebDriver chromeDriver, final String path) {
return chromeDriver.findElement(By.xpath(FORM_PATH + path));
}
private static List<WebElement> findFormElements(final WebDriver chromeDriver, final String path) {
return chromeDriver.findElements(By.xpath(FORM_PATH + path));
}
private static boolean hasPaceElements(final WebDriver chromeDriver) {
return !findFormElements(chromeDriver, PACE_PATH).isEmpty();
}
private static int findPaceValue(final WebDriver chromeDriver) {
final WebElement element = findFormElement(chromeDriver, PACE_PATH);
return Integer.parseInt(element.getAttribute("value"));
}
private static boolean hasTabElement(final WebDriver chromeDriver) {
return findFormElements(chromeDriver, TAB_PATH).size() == 1;
}
private static void sendAndClick(final WebDriver chromeDriver, final String key) {
findFormElement(chromeDriver, "b/b/table/tbody/tr[2]/td[2]/textarea").sendKeys("\"" + key + "\"");
findFormElement(chromeDriver, "b/b/table/tbody/tr[4]/td[2]/input[1]").click();
}
}
##Bug You talk about duplication of a block of code, but that code is not a duplicate in the source you posted. It's unclear if that's a bug or a posting error..
##Design Use methods! Even short, 1-line methods will make it much easier to read and understand what's going on. In particular, duplicated code should usually be extracted for clarity and maintenance sanity. Good method names can help document the intent of method calls. If the block of code you're talking about is supposed to be identical, you can extract it into a method whose name documents what that block of code does.
##General Don't abbreviate. Is "dBName" supposed to be "databaseName"? Nobody unfamiliar with your code will be able to tell what it is supposed to stand for.
For non-trivial applications, use a logger.
You pass in a non-final Cell parameter that is totally ignored. Don't ask for a cell if you're just going to overwrite it locally. You do know that the caller of this method isn't getting back the Cell you're working with inside the method, right?
i is a terrible name for a method argument. How about rowNumber?
Don't throw Exception. Throw the most specific exception(s) you can. If you've got to throw more than 2, create an abstraction-level exception (PaceException, maybe?), catch the other exceptions, and rethrow a PaceException.
It would be nice if there was an instance doing this work, instead of it happening in a static method. Then you wouldn't have to pass chromeDriver around all over the place.
Sample Refactoring
I didn't mess with the order of any of the calls because I wasn't sure where it was important. Some code can be further consolidated if there aren't any order dependencies.
import java.util.List;
public final class Snippet {
private static final String FORM_PATH = "html/body/form[2]/";
private static final String PACE_PATH = "table[1]/tbody/tr[2]/td[2]/input[1]";
private static final String TAB_PATH = "table[1]/tbody/tr[4]/td/b";
public static void main(final String[] argv) {
getPaceNumber(null, null, null, null, 0, null);
}
private static void getPaceNumber(
final WebDriver chromeDriver,
final String dbName,
final XSSFSheet paceSheet,
final String pubName,
final int rowNumber,
final XSSFWorkbook workbook) {
System.out.println("DBID is " + dbName + " and fpn is " + pubName);
final XSSFCell cell = paceSheet.getRow(rowNumber).createCell(1);
final int defaultHeight = paceSheet.getRow(0).getHeight();
paceSheet.getRow(rowNumber).setHeight((short) (defaultHeight * 2));
if (dbName == "" || dbName.equals("null")) {
System.out.println("Null Block");
cell.setCellValue("N/A");
searchPublication(chromeDriver);
return;
}
sendAndClick(chromeDriver, dbName);
int pubPaceNumber = 0;
int dbPaceNumber;
if (hasPaceElements(chromeDriver)) {
dbPaceNumber = findPaceValue(chromeDriver);
searchPublication(chromeDriver);
sendAndClick(chromeDriver, pubName);
if (hasPaceElements(chromeDriver)) {
pubPaceNumber = findPaceValue(chromeDriver);
} else {
if (hasTabElement(chromeDriver)) {
searchPublication(chromeDriver);
sendAndClick(chromeDriver, pubName);
}
cell.setCellValue("N/A");
}
if (dbPaceNumber == pubPaceNumber) {
cell.setCellValue(dbPaceNumber);
} else {
final CellStyle style = workbook.createCellStyle();
style.setWrapText(true);
style.setAlignment(CellStyle.ALIGN_RIGHT);
cell.setCellStyle(style);
cell.setCellValue(dbPaceNumber + "\n" + pubPaceNumber);
}
} else {
if (hasTabElement(chromeDriver)) {
searchPublication(chromeDriver);
sendAndClick(chromeDriver, pubName);
if (hasPaceElements(chromeDriver)) {
cell.setCellType(Cell.CELL_TYPE_NUMERIC);
cell.setCellValue(findPaceValue(chromeDriver));
} else {
cell.setCellValue("N/A");
}
} else {
cell.setCellValue("N/A");
}
}
searchPublication(chromeDriver);
}
private static void searchPublication(final WebDriver chromeDriver) {
chromeDriver.findElement(By.xpath(".//*[@id='searchPublication']")).click();
}
private static WebElement findFormElement(final WebDriver chromeDriver, final String path) {
return chromeDriver.findElement(By.xpath(FORM_PATH + path));
}
private static List<WebElement> findFormElements(final WebDriver chromeDriver, final String path) {
return chromeDriver.findElements(By.xpath(FORM_PATH + path));
}
private static boolean hasPaceElements(final WebDriver chromeDriver) {
return !findFormElements(chromeDriver, PACE_PATH).isEmpty();
}
private static int findPaceValue(final WebDriver chromeDriver) {
final WebElement element = findFormElement(chromeDriver, PACE_PATH);
return Integer.parseInt(element.getAttribute("value"));
}
private static boolean hasTabElement(final WebDriver chromeDriver) {
return findFormElements(chromeDriver, TAB_PATH).size() == 1;
}
private static void sendAndClick(final WebDriver chromeDriver, final String key) {
findFormElement(chromeDriver, "b/b/table/tbody/tr[2]/td[2]/textarea").sendKeys("\"" + key + "\"");
findFormElement(chromeDriver, "b/b/table/tbody/tr[4]/td[2]/input[1]").click();
}
}