Optimising part of a code thats bulky

malcomhfc

Hi there. I'm continuing to learn c# but moved on to the .net 4 compact framework. I have this piece of code i am using in a project. All though it does what i want it too, it feels to clunky and no need for multiple methods. I just don't know how to optimize it. I am trying to get it to do everything in the responsehandler, without the need for 3 of the same methods. It means my code wont be clunky and unoptimized and i believe all the feeds with display at once and not at different times.

/* removed, no need for it here i think */ but fyi its using system.xml and system.servicemodel.syndication --typo

namespace NewsTap
{
public partial class NewsFeed : PhoneApplicationPage
{
object _selectedItem;
public NewsFeed()
{

        InitializeComponent();
        SupportedOrientations = SupportedPageOrientation.Portrait;
        Loaded += new RoutedEventHandler(MainPage\_Loaded);

        PageTransitionList.Completed += new EventHandler(PageTransitionList\_Completed);

        // Set the data context of the listbox control to the sample data
        //DataContext = new MainViewModel();

        string bbc = "http://www.pchelpforum.com/external.php";
        string dailyMail = "http://www.dailymail.co.uk/home/index.rss";
        string guardian = "http://www.guardian.co.uk/tv-and-radio/rss";

        //bbc request
        HttpWebRequest bbcRequest = (HttpWebRequest)HttpWebRequest.Create(new Uri(bbc));
        bbcRequest.BeginGetResponse(new AsyncCallback(ResponseHandler), bbcRequest);

        //daily mail request
        HttpWebRequest DailyMailRequest = (HttpWebRequest)HttpWebRequest.Create(new Uri(dailyMail));
        DailyMailRequest.BeginGetResponse(new AsyncCallback(ResponseHandlerdaily), DailyMailRequest);
        // Guardian request
        HttpWebRequest GuardianRequest = (HttpWebRequest)HttpWebRequest.Create(new Uri(guardian));
        GuardianRequest.BeginGetResponse(new AsyncCallback(ResponseHandlerguardian), GuardianRequest);
        //Custom request -- coming soon

    }
    private void MainPage\_Loaded(object sender, RoutedEventArgs e)
    {
        // Reset page transition
        ResetPageTransitionList.Begin();
        
        
    }

    #region getting feed
    private void ListBbc\_MouseLeftButtonUp(object sender, MouseButtonEventArgs e)
    {
        // Capture selected item data

Gideon Engelberth

Well, lets see what's in common between your three methods. Everything is the same except for the address you read and which ItemsControl you are filling. If you combined those two things into a separate class, you could simplify the form code. The class would look something like this:

public class FeedReader
{
FeedReader(string address, ItemsControl target)
{
_address = address;
_target = target;
}

private string \_address;
private ItemsControl \_target;

public void LoadFeed()
{
    HttpWebRequest request = (HttpWebRequest)HttpWebRequest.Create(new Uri(\_address));
    request.BeginGetResponse(new AsyncCallback(ResponseHandler), request);
}

private void ResponseHandler(IAsyncResult asyncResult)
{
    HttpWebRequest request = (HttpWebRequest)asyncResult.AsyncState;
    HttpWebResponse response = (HttpWebResponse)request.EndGetResponse(asyncResult);

    if (response.StatusCode == HttpStatusCode.OK)
    {
        XmlReader reader = XmlReader.Create(response.GetResponseStream());
        SyndicationFeed newFeed = SyndicationFeed.Load(reader);
        \_target.Dispatcher.BeginInvoke(delegate
        {
            \_target.ItemsSource = newFeed.Items;
        });

    }
}

}

And the calling code could then be:

public NewsFeed()
{
InitializeComponent();
SupportedOrientations = SupportedPageOrientation.Portrait;
Loaded += new RoutedEventHandler(MainPage_Loaded);

PageTransitionList.Completed += new EventHandler(PageTransitionList\_Completed);

// Set the data context of the listbox control to the sample data
//DataContext = new MainViewModel();

//////////////////////////////
///// Here's the changes /////
//////////////////////////////
FeedReader bbc = new FeedReader("http://www.pchelpforum.com/external.php", ListBbc);
FeedReader dailyMail = new FeedReader("http://www.dailymail.co.uk/home/index.rss", ListDailyMail);
FeedReader guardian = new FeedReader("http://www.guardian.co.uk/tv-and-radio/rss", ListGuardian);

//just call this, no other routines needed for updating.
//all updates are taken care of by the FeedReader class methods.
bbc.LoadFeed();
dailyMail.LoadFeed();
guardian.LoadFeed();

//////////////////////////////
//////////////////////////////
//////////////////////////////

}

malcomhfc

Ah great modified it to my liking. Much shorter and more readable :D Thank you a lot. Sorry for my late reply was back to college this week. :(