Clock View in Android

Question

So I have built a clock widget. It basically has 2 hands and displays a time, either the current time or a selected time. There is another option which tells the clock if it should move around as the time does.

Is this a good way to do this? It seems to work well in my app but I was wondering if there might be some performance issues? What are the alternatives?

--EDIT--

I am aware that it's poor form to hard code lengths when drawing the lines. I will change that

public class ClockView extends RelativeLayout {

    private Animation minuteRotationAnimation;
    private Animation hourRotationAnimation;
    private LayoutInflater layoutInflater;
    private HandView minuteHandView;
    private HandView hourHandView;

    public ClockView(Context context, AttributeSet attrs) {
        super(context, attrs);
        layoutInflater = (LayoutInflater) context.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
        RelativeLayout relativeLayout = (RelativeLayout) layoutInflater.inflate(R.layout.clock_view, null);
        minuteHandView = (HandView) relativeLayout.findViewById(R.id.secondHandView);
        hourHandView = (HandView) relativeLayout.findViewById(R.id.hourHandView);
        hourHandView.setHeightPercentage(50);
       // hourHandView.setHalf(true);
        addView(relativeLayout);
        minuteRotationAnimation = AnimationUtils.loadAnimation(context, R.anim.rotate_minutes);
        hourRotationAnimation = AnimationUtils.loadAnimation(context, R.anim.rotate_hour);
    }

    public void setAsCurrentTime() {
        final Calendar cal = Calendar.getInstance();
        cal.setTimeInMillis(System.currentTimeMillis());
        Date date = cal.getTime();
        int hour = date.getHours();
        int minute = date.getMinutes();
        setTime(hour, minute, false);
    }

    public void setTime(long timestamp, boolean stop) {
        final Calendar cal = Calendar.getInstance();
        cal.setTimeInMillis(timestamp);
        Date date = cal.getTime();
        int hour = date.getHours();
        int minute = date.getMinutes();
        setTime(hour, minute, false);
    }

    public void setTime(int hour, int minute, boolean  stop) {
        int minuteDegrees = getDegrees(false, minute);
        int hourDegrees = getDegrees(true, hour);
        RotateAnimation hourStartAnimation = getRotateAnimation(hourDegrees);
        RotateAnimation minuteStartAnimation = getRotateAnimation(minuteDegrees);
        minuteHandView.startAnimation(minuteStartAnimation);

        if (stop == false) {
            AnimationSet hourAnimationSet = getAnimationSet(hourStartAnimation, hourRotationAnimation);
            AnimationSet minuteAnimationSet = getAnimationSet(minuteStartAnimation, minuteRotationAnimation);
            hourHandView.startAnimation(hourAnimationSet);
            minuteHandView.startAnimation(minuteAnimationSet);
        } else {
            hourHandView.startAnimation(hourStartAnimation);
            minuteHandView.startAnimation(minuteStartAnimation);
        }
    }

    private AnimationSet getAnimationSet(Animation animation1, Animation animation2) {
        AnimationSet animationSet = new AnimationSet(true);
        animationSet.addAnimation(animation1);
        animationSet.addAnimation(animation2);
        return animationSet;
    }

    private RotateAnimation getRotateAnimation(int degrees) {
        RotateAnimation animation = new RotateAnimation(0, degrees, 50, 50);
        animation.setDuration(1000);
        animation.setRepeatCount(0);
        animation.setRepeatMode(Animation.REVERSE);
        animation.setFillAfter(true);
        return animation;
    }

    private int getDegrees(boolean hour, int value) {
        int multiple;
        if (hour) {
            multiple = 12;
        } else {
            multiple = 60;
        }
        return (360 / multiple) * value;
    }
  }


public class HandView extends View {

    private Paint paint = new Paint();
    private int heightPercentage = 100;
    private boolean half = false;


    public void setHalf(boolean half) {
        this.half = half;
    }

    public HandView(Context context, AttributeSet attrs) {
        super(context, attrs);
        paint.setColor(context.getResources().getColor(R.color.clockBlue));
        paint.setStrokeWidth(5);
        paint.setAntiAlias(true);
    }

    public void setHeightPercentage(int heightPercentage) {
        this.heightPercentage = heightPercentage;
    }

    public void setWidth(float width) {
        paint.setStrokeWidth(width);
    }

    @Override
    public void onDraw(Canvas canvas) {
        super.onDraw(canvas);
        int height;
        if (half) {
            height = 30;
        } else {
            height = 10;
        }
        Rect rect = canvas.getClipBounds();
        canvas.drawLine(rect.exactCenterX(), rect.exactCenterY(), rect.exactCenterX(), height, paint);
    }
}

Answer 1

Just a few Java-related notes:

1. About getDegrees:

   Flag arguments are ugly. Passing a boolean into a function is a truly terrible practice. It immediately complicates the signature of the method, loudly proclaiming that this function does more than one thing. It does one thing if the flag is true and another if the flag is false!

   Source: Clean Code by Robert C. Martin, Chapter 3: Functions.

   I'd create two separate functions for hours and minutes:

   public int getMinuteDegrees(final int minutes) {
       final int multiplier = 60;
       return getDegrees(minutes, multiplier);
   }
   
   public int getHourDegrees(final int hours) {
       final int multiplier = 12;
       return getDegrees(hours, multiplier);
   }
   
   private int getDegrees(final int value, final int multiplier) {
       return (360 / multiplier) * value;
   }
   

   Notice the explanatory variable (multiplier) in both methods. References: Clean Code by Robert C. Martin, G19: Use Explanatory Variables; Refactoring: Improving the Design of Existing Code by Martin Fowler, Introduce Explaining Variable

2. According to the Code Conventions for the Java Programming Language, 3.1.3 Class and Interface Declarations, methods should be after constructors, so the setHalf() method should be after the constructor declaration.

3. The logic in setAsCurrentTime and setTime() almost the same. You could extract out a method here to remove the duplication:

   public void setAsCurrentTime() {
       long currentTimeMillis = System.currentTimeMillis();
       setTime(currentTimeMillis);
   }
   
   public void setTime(final long timestamp, final boolean stop) {
       setTime(timestamp);
   }
   
   private void setTime(final long timestamp) {
       final Calendar cal = Calendar.getInstance();
       cal.setTimeInMillis(timestamp);
       final Date date = cal.getTime();
       final int hour = date.getHours();
       final int minute = date.getMinutes();
       setTime(hour, minute, false);
   }
   

4. setTime(long timestamp, boolean stop) does not use the stop flag. It might be a bug but if it's just unnecessary remove it.