Pagination of repeater

I have some problems in my View.ascx.cs file because I'm reusing my code and modified it based on the situation. For example, when I apply pagination I have different code in all places. I just want my code to be simpler.

```
using System;
using DotNetNuke.Security;
using DotNetNuke.Services.Exceptions;
using DotNetNuke.Entities.Modules;
using DotNetNuke.Entities.Modules.Actions;
using DotNetNuke.Services.Localization;
using Christoc.Modules.ResourcesFilter.BOL;
using System.Data;
using System.Web.UI.WebControls;
using System.Collections.Generic;
using System.Web.Security;
using System.Web;
using DotNetNuke.Security.Permissions;
using System.Collections.Specialized;
using System.Web.UI;
using Christoc.Modules.ResourceModule.App_Code.BOL;

namespace Christoc.Modules.ResourcesFilter
{
/// -----------------------------------------------------------------------------
///
/// The View class displays the content
///
/// Typically your view control would be used to display content or functionality in your module.

public partial class View : ResourcesFilterModuleBase, IActionable
{
public ScriptManager sm;
protected override void OnInit(EventArgs e)
{
sm = ScriptManager.GetCurrent(this.Page);
sm.EnableHistory = true;
}

protected void Page_Load(object sender, EventArgs e)
{
try
{
this.ModuleConfiguration.ModuleTitle = "";
this.ModuleConfiguration.InheritViewPermissions = true;
if (!IsPostBack)
{
BindResourcesRepeater();
GetQueryString(HttpContext.Current.Request.Url);
}
}
catch (Exception exc) //Module failed to load
{
Exceptions.ProcessModuleLoadException(this, exc);
}
}

private void BindBookmarks()
{
DataSe

Your code duplication in BindResourceRepeater (2x in there) and btnSearch_Click is around the differences of how the keyword is handled and how the prev/next links are enabled/disabled.

Lets look at the link enabling first.

To me the logic of disabling the links should be very simple:

• The prev link should be enabled on every page except the first one

• The next link should be enabled on every page except the last one

Note that first and last page could be the same. The code should be two lines:

lnPrev.Visible = !objPds.IsFirstPage;
lnkNext.Visible = !objPds.IsLastPage;

I'd expect the PagedDataSource to set these flags correctly.

Then the only thing left is the keyword which should be simply passed as parameter and special cases handled in the calling function. This means we can now refactor it by moving the code into it's own function and call it in the appropriate places:

private void PaginateData(string keyword)
{
    DataSet ds = new DataSet();
    int selectedTopicID = Convert.ToInt32(ddlTopics.SelectedValue);
    int selectedSkillID = Convert.ToInt32(ddlSkills.SelectedValue);
    int selectedTypeID = Convert.ToInt32(ddlTypes.SelectedValue);
    int sortBy = Convert.ToInt32(ddlSortBy.SelectedValue);
    ds = Resource.Search_Resource(selectedTopicID, selectedSkillID, selectedTypeID, keyword, sortBy);

    PagedDataSource objPds = new PagedDataSource();
    objPds.DataSource = ds.Tables[0].DefaultView;
    objPds.AllowPaging = true;
    objPds.PageSize = 8;

    int curpage;
    if (ViewState["Page"] != null)
    {
        curpage = Convert.ToInt32(ViewState["Page"]);
    }
    else
    {
        ViewState["Page"] = 1;
        curpage = 1;
    }
    objPds.CurrentPageIndex = curpage - 1;

    rp_resList.DataSource = objPds;
    rp_resList.DataBind();

    lnkPrev.Visible = !objPds.IsFirstPage;
    lnkNext.Visible = !objPds.IsLastPage;

    int numberOfItems = ds.Tables[0].Rows.Count;
    lbl_totalResult.Text = GetCurrentVisibleItemsText(numberOfItems, objPds.PageSize, objPds.CurrentPageIndex);
}

BindResourceRepeater can be rewritten as:

private void BindResourcesRepeater()
{
    string tag = Request.QueryString["tag"];
    if (!String.IsNullOrEmpty(tag))
    {
        txtbKeyword.Text = tag.Trim();
    }
    else
    {
        tag = txtbKeyword.Text.Trim();
    }
    PaginateData(tag);
}

And btnSearch_Click can be rewritten as:

protected void btnSearch_Click(object sender, EventArgs e)
{
    ViewState["Page"] = 1;
    PaginateData(txtbKeyword.Text.Trim());
}

This refactoring got rid of approx. 130 lines of code.

A few other things:

• objPds is not a good name for a variable. Something like pagedData is better as it denotes what the variable represents.

• Likewise strArrTgas is a bad variable name. Incorporating the type name into the variables is not a standard C# naming convention is just creates noise most of the time.

• You look up a few controls by name and handle some commands. You do so by using "magic strings" (hard-coded strings scattered through the source). You should create const variables at the top of your class to hold these names so you can refactor them in one place should they ever change.

StackExchange Code Review Q#29963, answer score: 3