-3
\$\begingroup\$

I'm about to release this application as a hobby and I'm wondering how would I improve the speed of the calculations and overall responsiveness of the class?

I have covered a module briefly in data structures and algorithms so I'm thinking a big factor would be reducing the amount of loops I'm using.Can some suggest/pinpoint some improvements to the code or even a way of streamlining some of the operation? I'm thinking that I could be utilizing a better structure or design.This is the main class where the user input three calculations that are piped to a calculation.

public class MainActivity extends FragmentActivity implements OnClickListener{  

    SectionsPagerAdapter mSectionsPagerAdapter;

        /**
         * The {@link ViewPager} that will host the section contents.
         */
        private Vibrator myVib;

        ViewPager mViewPager;
        EditText offsetLength,offsetDepth,ductDepth;
        Button calculate;
        //DemoCollectionPagerAdapter mDemoCollectionPagerAdapter;
        String getoffsetlength; 
        String getoffsetdepth; 
        String getductdepth; 

        double off1,off2,off3;

        //button filter
        PorterDuffColorFilter greenFilter =  new PorterDuffColorFilter(Color.GREEN, PorterDuff.Mode.SRC_ATOP);

        @Override
        protected void onCreate(Bundle savedInstanceState) {
            super.onCreate(savedInstanceState);
            setContentView(R.layout.activity_main);

            final Intent intent1=new Intent(this,AboutActivity.class);
            final Intent intent2=new Intent(this,MainActivity.class);
            offsetLength = (EditText)findViewById(R.id.offLength);
            offsetDepth = (EditText)findViewById(R.id.offDepth);
            ductDepth = (EditText)findViewById(R.id.ductDepth);
            calculate = (Button)findViewById(R.id.calc);
            calculate.setOnClickListener(this);

            final ViewGroup actionBarLayout = (ViewGroup) getLayoutInflater().inflate(R.layout.a,null);

            // Set up your ActionBar
            final ActionBar actionBar = getActionBar();
            actionBar.setDisplayShowHomeEnabled(false);
            actionBar.setDisplayShowTitleEnabled(false);
            actionBar.setDisplayShowCustomEnabled(true);
            actionBar.setCustomView(actionBarLayout);

            //button vibration
            myVib = (Vibrator) this.getSystemService(VIBRATOR_SERVICE);

            // You customization
            final int actionBarColor = getResources().getColor(R.color.action_bar);
            actionBar.setBackgroundDrawable(new ColorDrawable(actionBarColor));

            final Button actionBarHome = (Button) findViewById(R.id.action_bar_title);
            actionBarHome.setBackgroundResource(R.drawable.ic_action_back);
            actionBarHome.setOnClickListener(this);
            actionBarHome.setOnClickListener(new View.OnClickListener() {

                @Override
                public void onClick(View view) {                

                   startActivity(intent2);

                }

            });
            //final Button actionBarSent = (Button) findViewById(R.id.action_bar_sent);

            final Button actionBarInfo = (Button) findViewById(R.id.action_bar_staff);
            actionBarInfo.setBackgroundResource(R.drawable.ic_action_help);
            actionBarInfo.setOnClickListener(this);
            actionBarInfo.setOnClickListener(new View.OnClickListener() {

                @Override
                public void onClick(View view) {                

                   startActivity(intent1);



                }

            });


            // Create the adapter that will return a fragment for each of the three
            // primary sections of the app.
            mSectionsPagerAdapter = new SectionsPagerAdapter(
                    getSupportFragmentManager());

            // Set up the ViewPager with the sections adapter.
            mViewPager = (ViewPager) findViewById(R.id.pager);
            mViewPager.setAdapter(mSectionsPagerAdapter);

        }

        @Override
        public void onClick(View v) {
            // TODO Auto-generated method stub

            try {

                double marking1=2,marking2=2,marking3=2;

                if (offsetLength.getText().length() > 0 ) {
                String getoffsetlength = offsetLength.getText().toString(); 
                }

                if (offsetDepth.getText().length() > 0 ) {
                String getoffsetdepth = offsetDepth.getText().toString(); 
                }
                if (ductDepth.getText().length() > 0 ) {
                String getductdepth = ductDepth.getText().toString(); 
                }
                double tri1,tri2;
                //double marking1,marking2,marking3;

                // validate whether the variables are not null
                if(getoffsetlength != null){
                    double off1 = Double.parseDouble(getoffsetlength);
                }
                if(getoffsetdepth != null){
                    double off2 = Double.parseDouble(getoffsetdepth);
                }
                if(getductdepth != null){
                double off3 = Double.parseDouble(getductdepth);
                }


                marking1 = Math.sqrt(Math.pow(off1,2) + Math.pow(off2,2));
                tri1 = Math.atan(off2 / off1);

                tri2 = (Math.PI - tri1) / 2;
                marking2 = off3 / Math.tan(tri2);

                marking3 = marking2 * 2;


                Intent myIntent = new Intent(MainActivity.this, CalcResult.class);
                myIntent.putExtra("number1", marking1);
                myIntent.putExtra("number2", marking2);
                myIntent.putExtra("number3", marking3);
                startActivity(myIntent);

                //vibrate button onClick
                myVib.vibrate(20);

                //calculate.setColorFilter(greenFilter);


                Toast.makeText(getBaseContext(), "Calculating!", Toast.LENGTH_SHORT).show(); 


            } catch (NumberFormatException e) {
                // TODO: handle exception
                System.out.println("Must enter a numeric value!");


            }
    //      
        }

        /*
        public boolean onCreateOptionsMenu(Menu menu) {
            // Inflate the menu; this adds items to the action bar if it is present.
            getMenuInflater().inflate(R.menu.main, menu);

            return true;
        }

        */

        @Override
        public void onBackPressed() {
            new AlertDialog.Builder(this, AlertDialog.THEME_HOLO_LIGHT)
                .setIcon(android.R.drawable.ic_dialog_alert)
                .setTitle("Closing Activity")
                .setMessage("Are you sure you want to close this activity?")
                .setPositiveButton("Yes", new DialogInterface.OnClickListener()
            {
                @Override
                public void onClick(DialogInterface dialog, int which) {
                    finish();    
                }

            })
            .setNegativeButton("No", null)
            .show();
        }

        /**
         * A {@link FragmentPagerAdapter} that returns a fragment corresponding to
         * one of the sections/tabs/pages.
         */
        public class SectionsPagerAdapter extends FragmentPagerAdapter {

            public SectionsPagerAdapter(FragmentManager fm) {
                super(fm);
            }

            @Override
            public Fragment getItem(int position) {
                // getItem is called to instantiate the fragment for the given page.
                // Return a DummySectionFragment (defined as a static inner class
                // below) with the page number as its lone argument.
                Fragment fragment = new DummySectionFragment();
                Bundle args = new Bundle();
                args.putInt(DummySectionFragment.ARG_SECTION_NUMBER, position + 1);
                fragment.setArguments(args);
                return fragment;
            }

            @Override
            public int getCount() {
                // Show 3 total pages.
                return 3;
            }

            @Override
            public CharSequence getPageTitle(int position) {
                Locale l = Locale.getDefault();
                switch (position) {
                case 0:
                    return getString(R.string.title_section1).toUpperCase(l);
                case 1:
                    return getString(R.string.title_section2).toUpperCase(l);
                case 2:
                    return getString(R.string.title_section3).toUpperCase(l);
                }
                return null;
            }
        }

        /**
         * A dummy fragment representing a section of the app, but that simply
         * displays dummy text.
         */
        public static class DummySectionFragment extends Fragment {
            /**
             * The fragment argument representing the section number for this
             * fragment.
             */
            public static final String ARG_SECTION_NUMBER = "section_number";

            public DummySectionFragment() {
            }

            @Override
            public View onCreateView(LayoutInflater inflater, ViewGroup container,
                    Bundle savedInstanceState) {
                View rootView = inflater.inflate(R.layout.fragment_main_dummy,
                        container, false);
                TextView dummyTextView = (TextView) rootView
                        .findViewById(R.id.section_label);
                dummyTextView.setText(Integer.toString(getArguments().getInt(
                        ARG_SECTION_NUMBER)));
                return rootView;
            }
        }

    }
\$\endgroup\$
  • 2
    \$\begingroup\$ The code you posted doesn't have any loops. As an additional comment, the repeated code in onClick() should be extracted into a single method that is called 3 times. \$\endgroup\$ – unholysampler Nov 25 '13 at 15:28
3
\$\begingroup\$

Looking through this code I feel that there must be something missing? Does it work? Does it even compile?

For example the onClick() method all the places I have marked are 'dead' variables... variables you declare inside a block and are not visible outside the block:

            double marking1=2,marking2=2,marking3=2;

            if (offsetLength.getText().length() > 0 ) {
            String getoffsetlength = offsetLength.getText().toString();  // MARK
            }

            if (offsetDepth.getText().length() > 0 ) {
            String getoffsetdepth = offsetDepth.getText().toString();  // MARK 
            }
            if (ductDepth.getText().length() > 0 ) {
            String getductdepth = ductDepth.getText().toString();  // MARK 
            }
            double tri1,tri2;
            //double marking1,marking2,marking3;

            // validate whether the variables are not null
            if(getoffsetlength != null){
                double off1 = Double.parseDouble(getoffsetlength);  // MARK
            }
            if(getoffsetdepth != null){
                double off2 = Double.parseDouble(getoffsetdepth);  // MARK
            }
            if(getductdepth != null){
            double off3 = Double.parseDouble(getductdepth);  // MARK
            }

Then, you have:

            marking1 = Math.sqrt(Math.pow(off1,2) + Math.pow(off2,2));
            tri1 = Math.atan(off2 / off1);

Firstly, these should fail to compile because the variables off1 and off2 are not in scope. Secondly, you should be zero-checking the division to ensure there's no division-by-zero.

All in all, I think this code is non-functional.....


EDIT

Ahh, I see that the variables I though were failing are actually declared at the class level, not in the method.

What this means is that your code is very buggy, and the lines I have marked above are COMPLETELY USELESS and have no impact on program execution....

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.