current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 4 down vote favorite

I have a problem with parsing RSS from a PHP page because the app is too slow.

This is my parsing code:

public void getdataparse(String url) {

    Log.d("Do in Background","Start Call Void");

    String dbtitle = "";
    String dbDesc = "";
    String dbimgicon = "";
    String dbimg = "";
    String dblink = "";
    String dbpubdate = "";
    String dborderStg = "";
    String dbsourcename = "";
    String dbimagecheck = "";
    int dbfff = 0;


        try {

            DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
            DocumentBuilder db = dbf.newDocumentBuilder();
            Document doc = db.parse(new URL(url).openStream());

            doc.getDocumentElement().normalize();

            NodeList nList = doc.getElementsByTagName("item");

            for (int temp = 0; temp < nList.getLength(); temp++) {


                dbtitle = "";
                dbDesc = "";
                dbimgicon = "";
                dbimg = "";
                dblink = "";
                dbpubdate = "";
                dborderStg = "";
                dbsourcename = "";
                dbimagecheck = "";
                dbfff = 0;
                Datemx = 0;
                Node nNode = nList.item(temp);
                if (nNode.getNodeType() == Node.ELEMENT_NODE) {

                    Element eElement = (Element) nNode;

                    dbtitle = getTagValue("title", eElement);

                    dbDesc = getTagValue("description", eElement);

                    String[] timeformated = new String[2];

                    timeformated = gettime(getTagValue("pubdate", eElement));

                    dbpubdate = timeformated[0];

                    dborderStg = timeformated[1];

                    Log.d("pubDate = ", "" + dbpubdate + "OrderStg = " + dborderStg);

                    dblink = getTagValue("link", eElement);


                    dbimgicon = getTagValue("sicon", eElement);

                    dbimg = "http://s14.postimg.org/cs10sq11d/whitemega.png";

                    dbsourcename = getTagValue("sname", eElement);

                    dbimagecheck = "";

                    dbfff = Datemx;

                    handler = new DataHandler(getBaseContext());
                    handler.open();
                    long id = handler.insertrssfeedData(dbtitle, dbDesc, dbimg, dbimgicon, dblink, dbpubdate, dbfff, dbsourcename , dbimagecheck);
                    handler.close();

                    Log.d("Add New Recorde","Record = " + temp + "/n Title = " + dbtitle);
                }
            }

        } catch (Exception e) {
            e.printStackTrace();
        }
    }

The PHP page URL

The PHP code

share|improve this question
    
If you want a review of your PHP code, you should ask a new question about that and include that code in the post itself. –  Simon Forsberg♦ May 30 '14 at 15:45
    
i think the php code is the problem –  user3680042 Jun 2 '14 at 6:36

2 Answers 2

up vote 5 down vote

none of those Variables need to be set to "" inside the for loop, they have already been set.

This can be set early and outside of the if block and the for loop:

 dbimg = "http://s14.postimg.org/cs10sq11d/whitemega.png";

Otherwise it is a Magic string that you will have to search for later.

The id isn't doing anything here, I don't think that you need it at all, you should be able to call the code like this

handler.insertrssfeedData(dbtitle, dbDesc, dbimg, dbimgicon, dblink, dbpubdate, dbfff, dbsourcename , dbimagecheck);

otherwise you are creating this variable and assigning a lot of data to it and never using it every time you go through the loop.

share|improve this answer
    
I am very grateful for your response and I will be following these tips and thank you –  user3680042 Jun 2 '14 at 6:39
    
@user3680042 you are welcome. I look forward to seeing more of your code in the future –  Malachi Jun 2 '14 at 13:06
up vote 5 down vote

Performance

I don't see any performance bottlenecks unfortunately.

Style and stuff

You're using a lot of empty lines, I would reduce the number of empty lines you have. You also use way too much lowercase in your variable names. (Although Datemx doesn't start with a small character, which it should).

It is hard to read text that is only written in lowercase and not space-separated, by using someBigCharactersHereAndThere it can be read more easily.

Some simple naming changes:

  • timeformated --> timeFormatted
  • dbimg --> dbImage
  • dbimagecheck --> dbImageCheck

Another Library

If you would use Jackson and it's XML-parsing capabilities, you would reduce a lot of your code. Jackson has the ability to parse XML and provide a Java object for it - if you first manage to create a class that has correct structure and annotations for the data with which Jackson should create it. I am not sure if using Jackson will make your code any faster or not, however.

The handler.insertrssfeedData method, or rather, handler.insertRssFeedData which you might want to rename it to, has a lot of parameters. By using the Jackson library, you will be able to reduce the number of parameters to only one.

Logging

The first parameter to Log.d should be a TAG that is used to categorize logging messages. Preferably, your entire application should use the same tag.

private static final String TAG = "MyApp";

Log.d(TAG, message);

This allows it to be filtered with adb's logcat command.

What good will calling e.printStackTrace(); do? It won't tell the user anything at runtime. Consider creating a dialog window that says what went wrong instead of simply printing the stack trace.

share|improve this answer
1  
+1 for Style and Stuff –  Malachi May 30 '14 at 18:24
    
I am very grateful for your response and I will be following these tips and thank you –  user3680042 Jun 2 '14 at 6:38

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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