HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Showing items and categories after clicking on a subcategory

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
aftershowingclickingitemscategoriesandsubcategory

Problem

How can I speed up my app? It's very slow if I click on a button and open a new page. All data in time is local. I'm not sure if the problem is in my code or xamarinu.

foreach (var category in avm.Categories) 
    {
        ToolbarItem ItemCategory = new ToolbarItem();
        ItemCategory.Name = category.Name;
        ToolbarItems.Add (ItemCategory);
        ItemCategory.Activated += (object sender, EventArgs e) => 
        {
            SubCategoryStack.Children.Clear();
            this.IsPresented = true;
            foreach (var Subcategory in category.SubCategories) 
            {
                Button btn_subcategory = new Button();
                btn_subcategory.Text = Subcategory.Name;
                SubCategoryStack.Children.Add(btn_subcategory);

                btn_subcategory.Clicked += (object senderr, EventArgs d) => 
                {   
                    this.IsPresented= false;
                    stackIbro2.Children.Clear();
                    stackIbro.Children.Clear();
                    Array array = Array.CreateInstance(typeof(Article), Subcategory.Articles.Count);
                    for(int i = 0; i ((Article[])array).ToString();

                };

            }
        };
    }

Solution

Naming

Assuming you or Mr.Maintainer will have to fix a bug in this code in 3 months, will you still know what any of these variables

  • avm



  • stackIbro2



  • stackIbro



  • articlesH



should do or represent ?

You should give your objects/variables meaningful names so that you or Mr.Maintainer will see at the first glance what to expect.

Indentation

StackLayout articlesH = new StackLayout()
                    {Orientation = StackOrientation.Horizontal};


here it can be missed at first glance that these 2 lines belong together. You should indent them like

StackLayout articlesH = new StackLayout()
                        {Orientation = StackOrientation.Horizontal};


btn_subcategory.Clicked eventhandler

  • Grouping together the related code, will increase readability.



  • the Array array is not really needed at all, you can remove it



  • commented code is dead code, hence it should be removed.



  • changing the for loop to a for each loop will add readability



  • EventArgs are name e and it is sender instead of senderr



-
don't use hungarian notation (at least like you have done). If the type changes but you forget to change the name also you get weird code like: Label btn_Quantity

btn_subcategory.Clicked += (object sender, EventArgs e) => 
{   
    this.IsPresented= false;
    stackIbro2.Children.Clear();
    stackIbro.Children.Clear();

    foreach (Article article in Subcategory.Articles)
    {
        StackLayout articlesH = new StackLayout()
            {Orientation = StackOrientation.Horizontal};

        Button button = new Button()
            {
                Text = article.Name,
                HorizontalOptions = LayoutOptions.StartAndExpand
            }

        Label label = new Label()
            {
                Text = article.Quantity.ToString(),
                HorizontalOptions = LayoutOptions.End
            }

        articlesH.Children.Add(button;
        articlesH.Children.Add(label);
        stackIbro2.Children.Add(articlesH);
    }
};

Code Snippets

StackLayout articlesH = new StackLayout()
                    {Orientation = StackOrientation.Horizontal};
StackLayout articlesH = new StackLayout()
                        {Orientation = StackOrientation.Horizontal};
btn_subcategory.Clicked += (object sender, EventArgs e) => 
{   
    this.IsPresented= false;
    stackIbro2.Children.Clear();
    stackIbro.Children.Clear();

    foreach (Article article in Subcategory.Articles)
    {
        StackLayout articlesH = new StackLayout()
            {Orientation = StackOrientation.Horizontal};

        Button button = new Button()
            {
                Text = article.Name,
                HorizontalOptions = LayoutOptions.StartAndExpand
            }

        Label label = new Label()
            {
                Text = article.Quantity.ToString(),
                HorizontalOptions = LayoutOptions.End
            }

        articlesH.Children.Add(button;
        articlesH.Children.Add(label);
        stackIbro2.Children.Add(articlesH);
    }
};

Context

StackExchange Code Review Q#78627, answer score: 3

Revisions (0)

No revisions yet.