5
\$\begingroup\$

My job is to make a specified java objects from web page that contains following text:

Companies:

[{"companyName":"Tire Systems","companyID":1},{"companyName":"IT Enterprise","companyID":2},{"companyName":"Car Manufacture","companyID":3},{"companyName":"Electro Market","companyID":4}]

Materials:

[{"name":"Winter Tire","ID":1,"companyID":1},{"name":"Summer Tire","ID":2,"companyID":1},{"name":"Tire","ID":3,"companyID":1},{"name":"Reporting Software","ID":4,"companyID":2},{"name":"Text Editor","ID":5,"companyID":2},{"name":"Passenger Car","ID":6,"companyID":3},{"name":"Truck","ID":7,"companyID":3},{"name":"Pickup","ID":8,"companyID":3},{"name":"Laptop","ID":9,"companyID":4},{"name":"Smartphone","ID":10,"companyID":4},{"name":"Tablet","ID":11,"companyID":4}]

This code took me a lot of time to be written and seems to be very complicated and non-efficient in my opinion. Here is source code with my description

private static String[][] pageContentToArray(String url, int attr) throws Exception {

    // getPageContent returns code like this [{"attribute":"value","attrubute":value},{"attr":"val","attr":val},...] 
    String[] pageContent = getPageContent(url).split(",");

    //len is attributes amount multipled by objects amount
    int len = pageContent.length;

    String[][] splitedPageContent = new String[len][attr];

    // len/attr returns number of objects
    String[][] groupedPageContent = new String[len/attr][attr];
    int savePoint = 0;
    for (int i = 0; i < len; ++i) {

        //removes all "{}[] characters
        splitedPageContent[i] = pageContent[i].replaceAll("[\\[\\](){}]","").replaceAll("\"","").split(":");

        // if statement is fullfiled every nth step where n equals attr value
        if (i % attr == (attr-1)) {
            int k = 0;

            //so basically it is reorganising data
            for (int j = attr-1; j >= 0; --j) {
                groupedPageContent[savePoint][k] = splitedPageContent[i-j][1];
                ++k;
            }
            ++savePoint;
        }
    }
    return groupedPageContent;
}

This is the basic method that taking page content (described in the beginning). Splits it to array by "," sign then removes all special characters like "{}[] then making 2d array from them by splitting with ":" char. There is an algorithm works "parallely" that every nth step (when n is amount of attributes) values are reorganized to new 2d array. The final product is an 2d array where first column contains attribute names and the second contains values.

public static Company[] pageToCompanies() {
    try {
        String[][] groupedPageContent = pageContentToArray(COMPANY_LIST, 2);
        int len = groupedPageContent.length;
        Company[] companies = new Company[len];
        for (int i = 0; i < len; ++i) {
            companies[i] = new Company(groupedPageContent[i][0], Integer.parseInt(groupedPageContent[i][1]));
        }
        return companies;
    } catch (Exception e) {
        e.printStackTrace();
        MainFrame.infoBox("Wrong companies page content");
    }
    return null; 

}

public static Material[] pageToMaterials() {
    String[][] groupedPageContent;
    try {
        groupedPageContent = pageContentToArray(MATERIAL_LIST, 3);
        int len = groupedPageContent.length;
        Material[] materials = new Material[len];
        for (int i = 0; i < len; ++i) {
            materials[i] = new Material(
                        groupedPageContent[i][0],
                        Integer.parseInt(groupedPageContent[i][1]),
                        Integer.parseInt(groupedPageContent[i][2])
                    );
        }
        return materials;
        } catch (Exception e) {
            e.printStackTrace();
            MainFrame.infoBox("Wrong materials page content");
    }
    return null; 
}

There are two methods that doing mostly the same thing grab values from 2d array and initialize objects with them. As you can see there is a lot of repeated code. First column of an array is not even used.

Possible Solution: Ideal solutions would be possibilities of:

  • passing class type to method as an argument
  • evaluate string to variable name just like it has been done in PHP or Bash Scripting (in these languages $var = "name" is String, but $$var is executable code)
  • some smart way for converting text to 2D array in one step, I feel that it can be done in one or few steps but I does not know how. I heard about Pattern class in java but still do not know if it is useful in this case

I think there is a way to completely rewrite for having 1 method instead of 3 and making code short, efficient with this solutions, however I am not sure if I would write it perfectly even with that knowledge so every example code will be greatly appreciated.

Or maybe you have another idea how to rewrite my code. I would be grateful for any help. Also sorry for my English it is not my native language.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ You do realize that the text youa re parsing is JSON data.... right? \$\endgroup\$
    – rolfl
    Commented May 22, 2015 at 20:09

1 Answer 1

5
\$\begingroup\$

You're working too hard. Although your post title says, "Create Java object from page content or plain text", actually it appears to be from JSON, specifically. Since it's such a common format, and it seems you're not actually trying to , you'd be much better off using one of the existing libraries for the task. For example Jackson seems popular.

Other than rewriting your program using Jackson or similar, some big issues that stand out:

  1. Catching Exception: you should catch the most specific exception type that can be thrown, Exception is way to generic. It doesn't give a clue about what can go wrong with the code, and it can catch other exception types that are truly unexpected.

  2. It seems you're overusing exceptions. For example in pageToCompanies, it's not clear what can throw that exception you're catching, and in any case it looks like you should be handling unexpected data with conditionals, not with exceptions.

\$\endgroup\$

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.