5
\$\begingroup\$

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
{
    /// -----------------------------------------------------------------------------
    /// <summary>   
    /// 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()
        {
            DataSet ds = new DataSet();
            Guid userID = Guid.Parse(Membership.GetUser(HttpContext.Current.User.Identity.Name).ProviderUserKey.ToString());
            ds = Bookmark.Get_all_Bookmarks_for_user(userID);
        }

        private void GetQueryString(Uri tempUri)
        {
            if (HttpUtility.ParseQueryString(tempUri.Query).Get("IntrestFocus") != null)
            {
                ddlTopics.SelectedValue = HttpUtility.ParseQueryString(tempUri.Query).Get("IntrestFocus"); 
            }
            else
                if (HttpUtility.ParseQueryString(tempUri.Query).Get("Skills") != null)
                {
                    ddlSkills.SelectedValue = HttpUtility.ParseQueryString(tempUri.Query).Get("Skills"); 
                }
            else
                if(HttpUtility.ParseQueryString(tempUri.Query).Get("Type") != null)
                {
                    ddlTypes.SelectedValue = HttpUtility.ParseQueryString(tempUri.Query).Get("Type"); 
                }    
        }

        public ModuleActionCollection ModuleActions
        {
            get
            {
                var actions = new ModuleActionCollection
                    {
                        {
                            GetNextActionID(), Localization.GetString("EditModule", LocalResourceFile), "", "", "",
                            EditUrl(), false, SecurityAccessLevel.Edit, true, false
                        }
                    };
                return actions;
            }
        }

        private void BindResourcesRepeater()
        {

            string tag = Request.QueryString["tag"];
            if (String.IsNullOrEmpty(tag))
            {

                //Guid userID = Guid.Parse(Membership.GetUser(HttpContext.Current.User.Identity.Name).ProviderUserKey.ToString());
                DataSet ds = new DataSet();
                int selectedTopicID = Convert.ToInt32(ddlTopics.SelectedValue);
                int selectedSkillID = Convert.ToInt32(ddlSkills.SelectedValue);
                int selectedTypeID = Convert.ToInt32(ddlTypes.SelectedValue);
                string keyword = txtbKeyword.Text.Trim();
                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;
                }
                // Set the currentindex
                objPds.CurrentPageIndex = curpage - 1;

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

                if (objPds.IsFirstPage)
                {
                    lnkPrev.Visible = false;
                    lnkNext.Visible = true;
                }
                else
                    if (!objPds.IsFirstPage && !objPds.IsLastPage)
                    {
                        lnkPrev.Visible = true;
                        lnkNext.Visible = true;
                    }

                else
                    if (objPds.IsLastPage)
                    {
                        lnkNext.Visible = false;
                        lnkPrev.Visible = true;
                    }
                int numberOfItems = ds.Tables[0].Rows.Count;
                lbl_totalResult.Text = GetCurrentVisibleItemsText(numberOfItems, objPds.PageSize, objPds.CurrentPageIndex);
            }
            else
            {               
                DataSet ds = new DataSet();
                int selectedTopicID = Convert.ToInt32(ddlTopics.SelectedValue);
                int selectedSkillID = Convert.ToInt32(ddlSkills.SelectedValue);
                int selectedTypeID = Convert.ToInt32(ddlTypes.SelectedValue);
                txtbKeyword.Text = tag;
                int sortBy = Convert.ToInt32(ddlSortBy.SelectedValue);
                ds = Resource.Search_Resource(selectedTopicID, selectedSkillID, selectedTypeID, tag, 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;
                }
                // Set the currentindex
                objPds.CurrentPageIndex = curpage - 1;

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

                if (objPds.IsFirstPage && objPds.Count < 8)
                {
                    lnkPrev.Visible = false;
                    lnkNext.Visible = false;

                }
                else if (objPds.IsFirstPage)
                {
                    lnkPrev.Visible = false;
                    lnkNext.Visible = true;
                }

                else if (!objPds.IsFirstPage && !objPds.IsLastPage)
                {
                    lnkPrev.Visible = true;
                    lnkNext.Visible = true;
                }    
                            else if (objPds.IsLastPage)
                {
                    lnkNext.Visible = false;
                    lnkPrev.Visible = true;
                }
                int numberOfItems = ds.Tables[0].Rows.Count;
                lbl_totalResult.Text = GetCurrentVisibleItemsText(numberOfItems, objPds.PageSize, objPds.CurrentPageIndex);
            }
        }

        private string GetCurrentVisibleItemsText(int numberOfItems, int pageSize, int currentPageIndex)
        {
            int startVisibleItems = currentPageIndex * pageSize + 1;
            int endVisibleItems = Math.Min((currentPageIndex + 1) * pageSize, numberOfItems);

            return string.Format(" {0}-{1} of {2} resources", startVisibleItems, endVisibleItems, numberOfItems);
        }

        protected void rp_resList_ItemDataBound(object sender, System.Web.UI.WebControls.RepeaterItemEventArgs e)
        {
            DataSet ds_bookmarkUser = null;
            DataSet ds_LikesUser = null;
            if (HttpContext.Current.User.Identity.IsAuthenticated)
            {
                ds_bookmarkUser = Bookmark.Get_all_Bookmarks_for_user(Guid.Parse(Membership.GetUser(HttpContext.Current.User.Identity.Name).ProviderUserKey.ToString()));
                ds_LikesUser = Like.Get_all_Likes_for_user(Guid.Parse(Membership.GetUser(HttpContext.Current.User.Identity.Name).ProviderUserKey.ToString()));
            }


             if (e.Item.ItemType == ListItemType.Item || e.Item.ItemType == ListItemType.AlternatingItem)
                {
                    Repeater childRepeater2 = (Repeater)e.Item.FindControl("rp_tagsTopics");
                    if (childRepeater2 != null)
                    {
                        //get the HiddenField Form parent Repeater rp_resList
                        HiddenField hf_resID = (HiddenField)e.Item.FindControl("hf_resID");
                        ImageButton imgBtn_bookmark = (ImageButton)e.Item.FindControl("imgBtn_bookmark");
                        LinkButton lb_like = (LinkButton)e.Item.FindControl("lb_like");
                        HyperLink hl_download = (HyperLink)e.Item.FindControl("hl_download");
                        LinkButton lnkBtnTags = (LinkButton)e.Item.FindControl("lnkBtnTags");

                        imgBtn_bookmark.Visible = HttpContext.Current.User.Identity.IsAuthenticated;
                        lb_like.Visible = HttpContext.Current.User.Identity.IsAuthenticated;
                        hl_download.Visible = HttpContext.Current.User.Identity.IsAuthenticated;

                        //if user is Authenticated 

                        int resID = Convert.ToInt32(hf_resID.Value);
                        //bind bookmark
                        if (ds_bookmarkUser != null)
                        {
                            foreach (DataRow row in ds_bookmarkUser.Tables[0].Rows)
                            {
                                if (resID == Convert.ToInt32(row["resourceID"]))
                                {
                                    imgBtn_bookmark.ImageUrl = "~/DesktopModules/ResourcesFilter/img/favorite-star-yellow.png";
                                    break;
                                }
                            }
                        }

                        //bind likes 
                        if (ds_LikesUser != null)
                        {
                            foreach (DataRow row in ds_LikesUser.Tables[0].Rows)
                            {
                                if (resID == Convert.ToInt32(row["resourceID"]))
                                {
                                    lb_like.Text = "Liked";
                                    break;
                                }
                            }
                        }

                        string[] strArrTgas = Resource.Get_Tags_For_Resource(resID);
                        if (strArrTgas[0] == " ")
                        {
                            childRepeater2.Visible = false;
                        }
                        else
                            childRepeater2.DataSource = strArrTgas;
                            childRepeater2.DataBind();
                    }
                }    
        }
        protected void btnSearch_Click(object sender, EventArgs e)
        {
            ViewState["Page"] = 1;
            DataSet ds = new DataSet();
            int selectedTopicID = Convert.ToInt32(ddlTopics.SelectedValue);
            int selectedSkillID = Convert.ToInt32(ddlSkills.SelectedValue);
            int selectedTypeID = Convert.ToInt32(ddlTypes.SelectedValue);
            string keyword = txtbKeyword.Text.Trim();
            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;
            }

            // Set the currentindex
            objPds.CurrentPageIndex = curpage - 1;

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

            //hide next & prev links 
         if (objPds.IsFirstPage && objPds.Count < 8)
            {
             lnkPrev.Visible = false;
             lnkNext.Visible = false;

            }
         else if (objPds.IsFirstPage && objPds.Count > 8)
            {
                lnkPrev.Visible = false;
                lnkNext.Visible = true;
            }
            else if (!objPds.IsFirstPage && !objPds.IsLastPage)
                {
                    lnkPrev.Visible = true;
                    lnkNext.Visible = true;
                }
                else if (objPds.IsLastPage)
                    {
                        lnkNext.Visible = false;
                        lnkPrev.Visible = true;
                    }

            //ViewState["Page"] = 1;

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

        protected void btnReset_Click(object sender, EventArgs e)
        {
            ViewState["Page"] = 1;
            ddlSkills.SelectedValue = "0";
            ddlTopics.SelectedValue = "0";
            ddlTypes.SelectedValue = "0";
            ddlSortBy.SelectedValue = "1";

            txtbKeyword.Text = "";
            lbl_totalResult.Text = "";
            BindResourcesRepeater();    
        }

        protected void LinkButton1_Click(object sender, EventArgs e)
        {
            LinkButton lnkBtnTags = (LinkButton)sender;
            Response.Redirect("~/WebsofWonder.aspx?tag=" + lnkBtnTags.Text);
        }

        protected void lnkPrev_Click(object sender, EventArgs e)
        {
            if (Convert.ToInt32(ViewState["Page"]) > 1)
            {
                // Set ViewStatevariable to the previous page
                ViewState["Page"] = Convert.ToInt32(ViewState["Page"]) - 1;
                // reload the control

            }
            sm.AddHistoryPoint("Currentpage", ViewState["Page"].ToString());
            BindResourcesRepeater();    
        }

        protected void lnkNext_Click(object sender, EventArgs e)
        {
            // Set ViewStatevariable to the next page
            ViewState["Page"] = Convert.ToInt32(ViewState["Page"]) + 1;
            sm.AddHistoryPoint("Currentpage", ViewState["Page"].ToString());
            // reload the control
            BindResourcesRepeater();
        }
        protected void rp_resList_ItemCommand(object source, RepeaterCommandEventArgs e)
        {
            //get userID
            Guid userID = Guid.Parse(Membership.GetUser(HttpContext.Current.User.Identity.Name).ProviderUserKey.ToString());

            if (e.CommandName == "bookmark_res")
            {
                //get resID 
                //get resource ID form HiddenField
                HiddenField hf = (HiddenField)e.Item.FindControl("hf_resID");
                ImageButton ib = (ImageButton)e.Item.FindControl("imgBtn_bookmark");
                int resID = Convert.ToInt32(hf.Value);

                //try to convert this block to fucntion 
                Bookmark bm = new Bookmark();

                bm.UserID = Guid.Parse(Membership.GetUser(HttpContext.Current.User.Identity.Name).ProviderUserKey.ToString());
                bm.Resource.ResourceID = resID; 

                //get bookmarkID to remove it from user bookmark and Group 
                int bookmarkID = Bookmark.Insert(bm);
                if (bookmarkID == -1)
                {
                    bool confirmDelete = Bookmark.Delete_User_bookmark(resID, userID);

                    ib.ImageUrl = "~/DesktopModules/ResourcesFilter/img/favorite-star.png";
                }
                else
                {
                    ib.ImageUrl = "~/DesktopModules/ResourcesFilter/img/favorite-star-yellow.png";

                }
            }
            if (e.CommandName == "lb_like_Click")
            {
                //get resID 
                HiddenField hf = (HiddenField)e.Item.FindControl("hf_resID");
                LinkButton lb_like = (LinkButton)e.Item.FindControl("lb_like");

                Like lke = new Like();
                lke.ResoursceID = Convert.ToInt32(hf.Value);
                lke.UserID = userID;
                int likeID = Like.Insert(lke);

                if (likeID == -1)
                {
                    bool confirmDelete = Like.Delete_User_Like(lke);
                    lb_like.Text = "Like";    
                }
                else
                {
                    lb_like.Text = "Liked";
                }
            }
        }
    }
}
\$\endgroup\$
2
  • \$\begingroup\$ you have a problem or the code works? \$\endgroup\$
    – Malachi
    Oct 29, 2013 at 21:02
  • \$\begingroup\$ there is a lot of code here, I think you might want to separate out some of the code that you want reviewed. like take out the most complicated part and post that asking for a review. don't post the whole code file, very few people are going to take the time to read through all the code. don't get me wrong there are going to be people that will read through it. but this hasn't gotten any answers yet. so I would trim and repost \$\endgroup\$
    – Malachi
    Oct 29, 2013 at 21:04

1 Answer 1

3
\$\begingroup\$

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.
\$\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.