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

Android Flashlight app

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

Problem

This is my first project, a Flashlight application for Android-based devices:

```
public class main extends Activity
{
private static final byte MENU_EXIT = 0, MENU_ABOUT = 1;
private static boolean FlashlightState;
private static long BackPressed;
private static ImageView Lamp;
private Flashlight FLASHLIGHT = null;

@Override
public void onCreate(Bundle savedInstanceState)
{
super.onCreate(savedInstanceState);
super.setContentView(R.layout.main);
super.getWindow().addFlags(WindowManager.LayoutParams.FLAG_FULLSCREEN);
FlashlightState = false;
Lamp = (ImageView) findViewById(R.id.imgVw_Lamp);
FLASHLIGHT = new Flashlight(this.getApplicationContext());

if (Flashlight.hasFlashlight) FLASHLIGHT.Open();

Lamp.setOnClickListener(new OnClickListener()
{
@Override
public void onClick(View arg0)
{
Lamp.setEnabled(false);

if (getSystemService(VIBRATOR_SERVICE) != null)
{
AudioManager VibratorState = (AudioManager) getApplicationContext().getSystemService(Context.AUDIO_SERVICE);
if (VibratorState.getRingerMode() != AudioManager.RINGER_MODE_SILENT)
{
Vibrator Switch = (Vibrator) getSystemService(Context.VIBRATOR_SERVICE);
Switch.vibrate(50);
}
VibratorState = null;
}

if (!FlashlightState) turnOnFlashlight();
else turnOffFlashlight();

Lamp.setEnabled(true);
return;
}
});

return;
}

@Override
protected void onStart()
{
super.onStart();
BackPressed = 0;

if (Flashlight.hasFlashlight) FLASHLIGHT.Open();

return;
}

@Override
protected void onPause()
{
super.onPause();

Solution

Use camelcase for your variable names.

Instead of:
public boolean MyBoolean write public boolean myBoolean

(This is to increase readability so you can tell the difference between class and object more clearly)
Use braces for if else statements

In your code you use:

if (!FlashlightState) turnOnFlashlight();
        else turnOffFlashlight();


More appropriate would be to use:

if(!FlashlightState) {
  turnOnFlashlight();
} else {
  turnOffFlashlight();
}


If you absolutely want to omit braces, omit them for single-statement solely-if clauses only. Such as:

if(bool) doSomething(); //No else statements
Using killProcess is a bad habit. (Destroy all your activities instead)

If you want to quit an App, just make sure all activities in the app are finish();'ed.
If you want to quit an activity, use finish();, this will call onDestroy() on your activity. You should let the OS handle the lifecycle of your activities as much as possible. Assuming that you have a single activity app, calling finish(); should destroy the only activity in your app, thus quitting it at the first opportunity.
Redundant return statements at the end of void functions.

Most of your functions have the statement return; in them at the end. This is completely redundant and serves no use.
Redundant final keyword in private method declarations

Your private methods are not going to be overridden or anything, they don't need the final keyword.

private: This method cannot be overridden by a subclass

final: This method cannot be overridden by a subclass

private final: This method cannot be overridden by a subclass and this method cannot be overridden by a subclass (Redundant)

Optional:
Refactor away your turnOn and turnOff functions.

You currently call two different functions that do a roughly similar thing depending on whether you toggle the flashlight on or off.

Instead of the following:

if (!FlashlightState) turnOnFlashlight();
        else turnOffFlashlight();


You could do something like:

toggleFlashLight();


In toggle flashlight you could do:

private void toggleFlashLight() {

  FlashlightState = !FlashlightState;

  if(FlashlightState) {
    //Turn it on;
  } else {
    //Turn it off;
  }
}


There's a dozen other ways to design it. (Where do you want to do the actual toggling of your boolean, etc. But this is just one example)

Code Snippets

if (!FlashlightState) turnOnFlashlight();
        else turnOffFlashlight();
if(!FlashlightState) {
  turnOnFlashlight();
} else {
  turnOffFlashlight();
}
if (!FlashlightState) turnOnFlashlight();
        else turnOffFlashlight();
toggleFlashLight();
private void toggleFlashLight() {

  FlashlightState = !FlashlightState;

  if(FlashlightState) {
    //Turn it on;
  } else {
    //Turn it off;
  }
}

Context

StackExchange Code Review Q#63012, answer score: 26

Revisions (0)

No revisions yet.