XmlPullParser with custom class doesn't show all RSS feeds

Question

I have the following custom class to fetch news from a few RSS feeds like New York Times, BBC, etc.:

class News{
   String title;
   String link;
   String imageURL;
}

This is the code I use to parse XML data:

 void getRSSList() {
    newsArray = new ArrayList<News>();

    // Load each RSS feed URL in a for loop
    for (int i = 0; i < catURLsList.size(); i++) {
         String feedURL = catURLsList.get(i);
         try {
             StrictMode.ThreadPolicy policy = new StrictMode.ThreadPolicy.Builder().permitAll().build();
             StrictMode.setThreadPolicy(policy);

             URL aUrl = new URL(feedURL);
             InputStream is = getInputStream(aUrl);
             parseRssFeeds(is);
          } catch (MalformedURLException e) { 
                 e.printStackTrace(); 
          }
     }
}




void parseRssFeeds(InputStream is)  {

        News n = new News();

        try {
            XmlPullParserFactory factory = XmlPullParserFactory.newInstance();
            factory.setNamespaceAware(false);
            XmlPullParser xpp = factory.newPullParser();
            xpp.setInput(is, "UTF_8");

            boolean insideItem = false;

            // Returns the type of current event: START_TAG, END_TAG, etc..
            int eventType = xpp.getEventType();
            while (eventType != XmlPullParser.END_DOCUMENT) {
                if (eventType == XmlPullParser.START_TAG) {

                    if (xpp.getName().equalsIgnoreCase("item")) {
                        insideItem = true;

                    // Get LINK
                    } else if (xpp.getName().equalsIgnoreCase("link")) {
                        if (insideItem) {
                            n.link = xpp.nextText();
                             Log.i("log-", "LINK: " + n.link);
                        }

                    // Get TITLE
                    } else if (xpp.getName().equalsIgnoreCase("title")) {
                        if (insideItem) {
                            n.title = xpp.nextText();
                             Log.i("log-", "TITLE: " + n.title);
                        }

                    // Get MEDIA URL
                    } else if (xpp.getName().equalsIgnoreCase("media:content")) {
                        if (insideItem)
                            n.imageURL = xpp.getAttributeValue(null, "url");
                             Log.i("log-", "MEDIA URL: " + n.imageURL);
                    }

                } else if (eventType == XmlPullParser.END_TAG && xpp.getName().equalsIgnoreCase("item")) {
                    insideItem = false;
                }

                // Add news objects to the newsArray
                newsArray.add(n);

                eventType = xpp.next(); // move to next element

            } // end WHILE loop

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


        setNewsGridView();
    }

setNewsGridView() calls a method with a GridAdapter inside which shows the title and image of RSSfeeds, the problem is that i get all titles, links and media URL's in my Logcat, but I get only the one news feed repeated in each cell of my GridView, as may times as the newsArray's size.

This is my GridAdapter:

 // MARK: - SET NEWS GRID VIEW ---------------------------------------------
    void setNewsGridView() {

        class GridAdapter extends BaseAdapter {
            private Context context;
            public GridAdapter(Context context, List<News> objects) {
                super();
                this.context = context;
            }


            // CONFIGURE CELL
            @Override
            public View getView(int position, View cell, ViewGroup parent) {
                if (cell == null) {
                    LayoutInflater inflater = (LayoutInflater) context.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
                    cell = inflater.inflate(R.layout.cell_news, null);
                }

                // Get News object
                News n = newsArray.get(position);


                // Get Title
                TextView titleTxt = (TextView) cell.findViewById(R.id.cnTitleTxt);
                titleTxt.setText(n.title);


                // Get Image
                ImageView newsImg = (ImageView)cell.findViewById(R.id.cnImage);
                if (n.imageURL != null) {
                    Picasso.with(context).load(n.imageURL).into(newsImg);
                } else { newsImg.setImageResource(R.drawable.logo); }



                return cell;
            }

            @Override public int getCount() { return newsArray.size(); }
            @Override public Object getItem(int position) { return newsArray.get(position); }
            @Override public long getItemId(int position) { return position; }
        }


        // Init GridView and set its adapter
        GridView aGrid = (GridView) findViewById(R.id.hNewsGridView);
        aGrid.setAdapter(new GridAdapter(Home.this, newsArray));

        // Set number of Columns accordingly to the device used
        float scalefactor = getResources().getDisplayMetrics().density * 150; // 150 is the cell's width
        int number = getWindowManager().getDefaultDisplay().getWidth();
        int columns = (int) ((float) number / (float) scalefactor);
        aGrid.setNumColumns(columns);


    }

My Logcat when I run the app:

    I/log-: TITLE: Senate Votes Down Broad Obamacare Repeal
    I/log-: LINK: https://www.nytimes.com/2017/07/25/us/politics/senate-health-care.html?partner=rss&emc=rss
    I/log-: MEDIA URL: https://static01.nyt.com/images/2017/07/26/us/26dc-health-sub1/26dc-health-sub1-moth.jpg
    I/log-: TITLE: John McCain to Senate: ‘We’re Getting Nothing Done’
    I/log-: LINK: https://www.nytimes.com/video/us/politics/100000005305566/john-mccain-health-bill-vote.html?partner=rss&emc=rss
    I/log-: TITLE: McCain Returns to Cast Vote to Help the President Who Derided Him
    I/log-: LINK: https://www.nytimes.com/2017/07/25/us/politics/mccain-health-care-brain-cancer

etc...
...

And this is the output I get on the device:

What am I doing wrong?

Thanks so much!

hugh · Accepted Answer · 2017-07-30T07:59:48.553

Inside parseRssFeeds you instantiate a News object before entering the main loop, then when you extract a news block from your XML, you set values on this and add it to newsArray. Then as you continue iterating, you overwrite the values in the News object n each time. Since Java passes objects as references, you end up with a list containing multiple copies of a single instance, n, whose values are set to the last entry in your XML.

To fix this, you need to move the declaration inside the loop. (edit) The problem with my first attempt here was I assumed each News object would be read during a single loop iteration, which isn't the case. You need to instantiate a new News object each time you encounter a new news item, which should give you something like the following:

            // Returns the type of current event: START_TAG, END_TAG, etc..
            News currentNewsItem = null;
            int eventType = xpp.getEventType();
            while (eventType != XmlPullParser.END_DOCUMENT) {

                if (eventType == XmlPullParser.START_TAG) {
                    if (xpp.getName().equalsIgnoreCase("item")) {
                        insideItem = true;

                    // Get LINK
                    } else if (xpp.getName().equalsIgnoreCase("link")) {
                        if (insideItem) {
                            // If no item is currently in progress, start one
                            currentNewsItem = startNewItemIfRequired(currentNewsItem, newsArray);

                            currentNewsItem.link = xpp.nextText();
                             Log.i("log-", "LINK: " + currentNewsItem.link);
                        }

                    // Get TITLE
                    } else if (xpp.getName().equalsIgnoreCase("title")) {
                        if (insideItem) {
                            // Start a new news item, even if one is already in progress 
                            currentNewsItem = startNewItemIfRequired(null, newsArray);

                            currentNewsItem.title = xpp.nextText();
                             Log.i("log-", "TITLE: " + currentNewsItem.title);
                        }

                    // Get MEDIA URL
                    } else if (xpp.getName().equalsIgnoreCase("media:content")) {
                        if (insideItem)
                            // If no item is currently in progress, start one
                            currentNewsItem = startNewItemIfRequired(currentNewsItem, newsArray);

                            currentNewsItem.imageURL = xpp.getAttributeValue(null, "url");
                             Log.i("log-", "MEDIA URL: " + currentNewsItem.imageURL);
                    }

                } else if (eventType == XmlPullParser.END_TAG && xpp.getName().equalsIgnoreCase("item")) {
                    insideItem = false;
                }

                eventType = xpp.next(); // move to next element

            } // end WHILE loop

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

With this additional function:

private News startNewItemIfRequired(News currentNewsItem, List<News> newsArray) {
    if (currentNewsItem==null){
        currentNewsItem = new News();
        newsArray.add(currentNewsItem);
    }
    return currentNewsItem;
}

This will create a new News item whenever a "title" start tag is encountered, and add it to the panel, then continue populating this until a new one is begun. For safety, I've added in some checks which will guard against null pointers if there is no "title". That said, this does add a constraint that "title" must appear first in the XML you receive - this appears to be the case from your logs, but if you expect this to vary you will need something a little more complex.

I'd add that this kind of streaming approach is the simplest way to get this started, but if you want to make this more robust in future (particularly guarding against changes in XML structure and missing fields), you might be better served by reading the XML as a document first, then pulling out the structures you're interested in.

I'm afraid I haven't your question hasn't got enough code for me to stand this up without spending some time picking apart your dependencies, so I haven't been able to actually test that code myself, but hopefully that will give you enough to go on to fix your problem!

it doesn't work :( also, "if (n != null)" is always true so it's not needed. If I use "News n = new News()" it gives me empty cells in my GridView, instead if I use "News n = null;" it gives me no cells at all. — Frank Eno, Jul 30 '17 at 04:58
ok i'll try again later, i noticed that it doesn't even print the logs today and it keeps printing an SQL error even if i don't use SQL at all in my app... — Frank Eno, Jul 30 '17 at 07:36
n would be null if the eventType is a START_TAG of type "item" or a non-START_TAG, because it starts off as null and is never populated. When you say it doesn't work, what do you get? Possibly lots of partially populated panels, some with links, some titles and some URLs? — hugh, Jul 30 '17 at 07:37
i get lots of cells with no title and images, like if the newsArray as a size but null data. but what's even curious is that i get a few line about this: https://stackoverflow.com/questions/39735044/sqliteconnection-databases-leak-when-running-emulator i've tried doing what's suggested in the answers but no success :( — Frank Eno, Jul 30 '17 at 07:45
Ok, I think I misunderstood what you were trying to do - I'd assumed link/image/etc types were alternatives. Have updated my answer with a change which should address this. — hugh, Jul 30 '17 at 08:01
thanks again, i marked the other answer as accepted because it has a simpler code and it works smoothly, i usually choose the simplest solutions with less code as possible, no extra functions (although sometimes they're needed, i know). — Frank Eno, Jul 30 '17 at 08:56
It's you choice of course, but the extra complexity I've added is a check to prevent a NPE if the first item doesn't have a title. In my opinion, the added code is justified because it makes your application more robust - simplicity is good, but not at the price of being fragile and error-prone. — hugh, Jul 30 '17 at 09:24
yep, your code works well too, none of my RSS feeds is missing the title tag but it's still great to have a check for null value. thanks again! — Frank Eno, Aug 03 '17 at 04:01

Muthukrishnan Rajendran · Answer 2 · 2017-07-30T08:13:07.960

The problem is you created News n = new News(); object one time but adding that object into ArrayList every time, so it's always showind the last news content in all the item. You should create the object every time when ever you get new tag called item, and int the end of the tag item, you should insert into the ArrayList.

You should do like this,

void parseRssFeeds(InputStream is) {

    // News n = new News(); your are creating object only here.
    News n = null;

    try {
        XmlPullParserFactory factory = XmlPullParserFactory.newInstance();
        factory.setNamespaceAware(false);
        XmlPullParser xpp = factory.newPullParser();
        xpp.setInput(is, "UTF_8");

        boolean insideItem = false;

        // Returns the type of current event: START_TAG, END_TAG, etc..
        int eventType = xpp.getEventType();
        while (eventType != XmlPullParser.END_DOCUMENT) {
            if (eventType == XmlPullParser.START_TAG) {

                if (xpp.getName().equalsIgnoreCase("item")) {
                    insideItem = true;
                    n = new News(); // you should intialize the object every time here

                    // Get LINK
                } else if (xpp.getName().equalsIgnoreCase("link")) {
                    if (insideItem) {
                        n.link = xpp.nextText();
                        Log.i("log-", "LINK: " + n.link);
                    }

                    // Get TITLE
                } else if (xpp.getName().equalsIgnoreCase("title")) {
                    if (insideItem) {
                        n.title = xpp.nextText();
                        Log.i("log-", "TITLE: " + n.title);
                    }

                    // Get MEDIA URL
                } else if (xpp.getName().equalsIgnoreCase("media:content")) {
                    if (insideItem)
                        n.imageURL = xpp.getAttributeValue(null, "url");
                    Log.i("log-", "MEDIA URL: " + n.imageURL);
                }

            } else if (eventType == XmlPullParser.END_TAG && xpp.getName().equalsIgnoreCase("item")) {
                insideItem = false;
                // End of the item tag, we should add it into the list
                // Add news objects to the newsArray
                newsArray.add(n);
            }


            eventType = xpp.next(); // move to next element

        } // end WHILE loop

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


    setNewsGridView();
}

score 0 · Answer 3 · answered Jul 31 '17 at 00:24

Retrofit with SimpleXmlConverter

          OkHttpClient okHttpClient = new 
          OkHttpClient.Builder().readTimeout(60, 
          TimeUnit.SECONDS).connectTimeout(60, TimeUnit.SECONDS)
            .addInterceptor(new Interceptor() {
        @Override
        public Response intercept(Interceptor.Chain chain) throws IOException 
         {
            Request original = chain.request();

            Request.Builder requestBuilder = original.newBuilder()
                    .header("Accept", "application/xml");

            Request request = requestBuilder.build();
            return chain.proceed(request);
        }
    }).build();

    retrofitClient = new Retrofit.Builder()
            .baseUrl(RestAPI.baseURL)
            .client(okHttpClient)
            .addConverterFactory(SimpleXmlConverterFactory.create())
            .build();

    restAPI = retrofitClient.create(RestAPI.class);

XmlPullParser with custom class doesn't show all RSS feeds

3 Answers3