3 Replies Latest reply on Aug 15, 2007 7:23 PM by James Bower

    Writing cleaner more efficient code

    James Bower Level 1
      I have a loginResultHandler that separates 4 roles roles and 14 or so branches of a company. I have a lot of code that repeats the same functions for basic user roles for each branch that I'd like to state once instead of 14 times if possible. Here's a sample:

      private function loginResultHandler(event:ResultEvent):void
      {
      currentUser = event.result as User;


      if (currentUser.loggedIn && currentUser.isEmployee)
      {

      // If login successful
      /* Administrator View */
      if (currentUser.roles == "3")
      {
      theMenuBar.dataProvider = adminMenu;
      theControlBar.dock = true;
      appView.y=0;
      websiteBtn.visible = true;
      websiteBtn.includeInLayout = true;
      adminsiteBtn.visible = false;
      adminsiteBtn.includeInLayout = false;
      }

      /* Manager View */
      else if (currentUser.roles == "2")
      {
      theMenuBar.dataProvider = managersMenu;
      theControlBar.dock = true;
      appView.y=0;
      websiteBtn.visible = true;
      websiteBtn.includeInLayout = true;
      adminsiteBtn.visible = false;
      adminsiteBtn.includeInLayout = false;
      }

      /* User Views */
      /* Head Office 1 *//* Branch 2*//* Branch 3*//* Branch 4*//* Branch 5*/
      else if (currentUser.roles == "1" && (currentUser.branch == "1" ||
      currentUser.branch == "2" || currentUser.branch == "3" ||
      currentUser.branch == "4" || currentUser.branch == "5"))
      {
      theMenuBar.dataProvider = branchMenu;
      theControlBar.dock = false;
      theControlBar.width = 1010;
      appView.y=32;
      appView.selectedChild = homeViewBranch;
      adminsiteBtn.visible = false;
      adminsiteBtn.includeInLayout = false;
      websiteBtn.visible = true;
      websiteBtn.includeInLayout = true;
      }
      /* WestCoast */
      else if (currentUser.roles == "1" && currentUser.branch == "6")
      {
      theMenuBar.dataProvider = westCoastMenu;
      theControlBar.dock = false;
      theControlBar.width = 1010;
      appView.y=32;
      appView.selectedChild = homeViewWestCoast;
      adminsiteBtn.visible = false;
      adminsiteBtn.includeInLayout = false;
      websiteBtn.visible = true;
      websiteBtn.includeInLayout = true;
      }
      /* EastCoast */
      else if (currentUser.roles == "1" && currentUser.branch == "7")
      {
      theMenuBar.dataProvider = eastCoastMenu;
      theControlBar.dock = false;
      theControlBar.width = 1010;
      appView.y=32;
      appView.selectedChild = homeViewEastCoast;
      adminsiteBtn.visible = false;
      adminsiteBtn.includeInLayout = false;
      websiteBtn.visible = true;
      websiteBtn.includeInLayout = true;
      }
      /* NorthRegion */
      else if (currentUser.roles == "1" && currentUser.branch == "8")
      {
      theMenuBar.dataProvider = northRegionMenu;
      theControlBar.dock = false;
      theControlBar.width = 1010;
      appView.y=32;
      appView.selectedChild = homeViewNorthRegion;
      adminsiteBtn.visible = false;
      adminsiteBtn.includeInLayout = false;
      websiteBtn.visible = true;
      websiteBtn.includeInLayout = true;
      }
      ...and so on for another 11 branches.

      The only thing that differs here is each branch's home view. Of course the admin and manager menus also differ but the code there is basically fine. I'd like to be able to compact the following:


      theControlBar.dock = false;
      theControlBar.width = 1010;
      appView.y=32;
      adminsiteBtn.visible = false;
      adminsiteBtn.includeInLayout = false;
      websiteBtn.visible = true;
      websiteBtn.includeInLayout = true;

      since it repeats for each branch. Also would I be better off using switch statements instead of else if in this case?

      Thanks for your help.

      James
        • 1. Re: Writing cleaner more efficient code
          JoshBeall Level 1
          quote:

          Originally posted by: James Bower
          theControlBar.dock = false;
          theControlBar.width = 1010;
          appView.y=32;
          adminsiteBtn.visible = false;
          adminsiteBtn.includeInLayout = false;
          websiteBtn.visible = true;
          websiteBtn.includeInLayout = true;

          since it repeats for each branch. Also would I be better off using switch statements instead of else if in this case?

          Thanks for your help.

          James



          Is there a reason you can't just place that code before your if/else if blocks?

          In regards to switch: switch works great when you are only checking a single criterium per case statement. You have multiple criteria in some of your if() blocks. I'm not aware of a way to have a case statement (used in a switch block) that checks multiple criteria. I'm a C#'er, though, and I have yet to learn some everything there is to know about ActionScript.

          -Josh
          • 2. Re: Writing cleaner more efficient code
            levancho Level 3
            this could be one way to eliminate redundancy :


            -----------------------------


            private function loginResultHandler(event:ResultEvent):void
            {
            currentUser = event.result as User;


            if (currentUser.loggedIn && currentUser.isEmployee)
            {

            // If login successful
            /* Administrator View */
            if (currentUser.roles == "3" || currentUser.roles == "2")
            {
            theMenuBar.dataProvider =currentUser.roles == "3"? adminMenu:managersMenu;
            theControlBar.dock = true;
            appView.y=0;
            websiteBtn.visible = true;
            websiteBtn.includeInLayout = true;
            adminsiteBtn.visible = false;
            adminsiteBtn.includeInLayout = false;

            }

            /* User Views */
            /* Head Office 1 *//* Branch 2*//* Branch 3*//* Branch 4*//* Branch 5*/
            else if (currentUser.roles == "1"){

            if(currentUser.branch == "1" ||
            currentUser.branch == "2" || currentUser.branch == "3" ||
            currentUser.branch == "4" || currentUser.branch == "5"))
            {
            theMenuBar.dataProvider = branchMenu;
            appView.selectedChild = homeViewBranch;
            }
            /* WestCoast */
            else if (currentUser.branch == "6")
            {
            theMenuBar.dataProvider = westCoastMenu;
            appView.selectedChild = homeViewWestCoast;

            }
            /* EastCoast */
            else if (currentUser.branch == "7")
            {
            theMenuBar.dataProvider = eastCoastMenu;
            appView.selectedChild = homeViewEastCoast;

            }
            /* NorthRegion */
            else if (currentUser.branch == "8")
            {
            theMenuBar.dataProvider = northRegionMenu;
            appView.selectedChild = homeViewNorthRegion;
            }
            // etc 67 more ...

            theControlBar.dock = false;
            theControlBar.width = 1010;
            appView.y=32;
            adminsiteBtn.visible = false;
            adminsiteBtn.includeInLayout = false;
            websiteBtn.visible = true;
            websiteBtn.includeInLayout = true;

            }// end of --> else if (currentUser.roles == "1")
            -----------------
            • 3. Re: Writing cleaner more efficient code
              James Bower Level 1
              Josh and levancho thanks for the lessons in lean coding. I especially liked the boolean for determining which menu was used in the admin. With your help I could eliminated a few hundred lines of fat and learn some cool AS3 coding.

              Cheers,

              James