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

Transfer XML data to database where table fields must match with elements

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

Problem

This is my first Java program, where I have a class that gets XML data from a certain website and another class that does all the database related tricks. I omitted all non-related code to this case.

So the idea is simply to get the field names in the database table, then use the retrieved XML's content to fill an array matching the table fields index's. After that's done, insert that data into the database.

Having less elements in the XML than fields in the database table is permitted (as long as the names match), but having more elements in the XML than in the database is not and the entry should be denied. The program should not crash, however.

My question is how can this be improved? Security, performance...?

First, I have a class called XML.java, that gets the data I need:

```
import java.io.IOException;
import org.jsoup.Jsoup;
import org.jsoup.nodes.Document;
import org.jsoup.select.Elements;

public class XML {

Database database = new Database();

public void searchAuctions(String searchQuery) throws IOException {

String serviceAPI = "www.url.com?par=" + searchQuery;

Document xmlResponse = Jsoup.connect(serviceAPI).get();

Elements items = xmlResponse.select("item");

try {

String fields[] = database.getFields("databaseName", "tableName");
String values[] = new String[fields.length];
values[0] = null;

for (int i = 0; i < items.size(); i++) {
Elements product = items.eq(i);
for(int j = 1; j < fields.length; j++) {
values[j] = product.select(fields[j]).text();
}
boolean isInsert = database.insert("databaseName", "tableName", fields, values);
if(!isInsert) {
System.out.println("Failed to add entry.");
}
}

} catch (Exception ex) {
ex.printStackTrace(System.out);
System.exit(1);
}

Solution

private static String dbUsername = "username";
        private static String dbPassword = "password";

        private static String databaseAPI = "jdbc:mysql://localhost:3306/databaseName";


You could make all these final, as they shouldn't change during the course of running the program.

private static Connection connection;
        private static Statement command;


Why are these class fields? They could be local variables. So

try {
                connection = DriverManager.getConnection(databaseAPI, dbUsername, dbPassword);
                command = connection.createStatement();


could be

try (Connection connection = DriverManager.getConnection(databaseAPI, dbUsername, dbPassword)) {
                Statement command = connection.createStatement();


That restricts the variables to local scope.

Also, this switches to the try-with-resources form of the try statement as being more appropriate when your first statement opens something.

String separatedFields = "";
            String statementParameters = "";

            for(int i = 0; i < fields.length; ++i) {
                if(i != fields.length - 1) {
                    separatedFields = separatedFields.concat(fields[i] + ",");
                    statementParameters = statementParameters.concat("?,");
                } else {
                    separatedFields = separatedFields.concat(fields[i]);
                    statementParameters = statementParameters.concat("?");
                }
            }


You can save a comparison per loop iteration by rewriting this as

String separatedFields = fields[0];
            String statementParameters = "?";
            for (int i = 1; i < fields.length; ++i) {
                separatedFields = separatedFields.concat(", " + fields[i]);
                statementParameters = statementParameters.concat(", ?");
            }


I also added some additional whitespace for easier reading.

And note how this saves initializing to blank strings.

It removes the need to subtract one from the length and to check on every iteration of the loop if it's the last.

One reason this works in this case is that you must have at least one value in fields for the SQL statement to run properly.

Code Snippets

private static String dbUsername = "username";
        private static String dbPassword = "password";

        private static String databaseAPI = "jdbc:mysql://localhost:3306/databaseName";
private static Connection connection;
        private static Statement command;
try {
                connection = DriverManager.getConnection(databaseAPI, dbUsername, dbPassword);
                command = connection.createStatement();
try (Connection connection = DriverManager.getConnection(databaseAPI, dbUsername, dbPassword)) {
                Statement command = connection.createStatement();
String separatedFields = "";
            String statementParameters = "";

            for(int i = 0; i < fields.length; ++i) {
                if(i != fields.length - 1) {
                    separatedFields = separatedFields.concat(fields[i] + ",");
                    statementParameters = statementParameters.concat("?,");
                } else {
                    separatedFields = separatedFields.concat(fields[i]);
                    statementParameters = statementParameters.concat("?");
                }
            }

Context

StackExchange Code Review Q#126533, answer score: 2

Revisions (0)

No revisions yet.