Can you review this for ease of understanding? (Drawing clock hands)

Question

0.00/5 (No votes)

See more:

I've built a graphics library and I'm coming up with some examples.

The trick is making them understandable. I can't judge my own code, like my API for usability because I wrote it.

The test of code, IMO, is how self evident it is. I'd like you to look at a section of code in "what I have tried" and tell me how you feel in terms of its readability, and what could be improved. If the API seems complicated I want to know that especially.

There is a weirdness I want to explain that I explain somewhat in my documentation, and that is the templatization of this function.

GFX doesn't use inheritance. It binds at the source level, not the binary level, using templates. Destination& dst is a draw target - think of it as a canvas of sorts. That way you can pass something in and it will draw to that.

What I have tried:

C++

// draw analog clock
// does the hands only
template<typename Destination>
void draw_clock(Destination& dst, tm& time,const srect16& bounds) {
  srect16 b = bounds.normalize();
  uint16_t w = min(b.width(),b.height());


  // using a viewport to do our rotation
  // but we're not going to draw to it
  // instead we just use its rotate
  // and translate features.
  using view_t = viewport<Destination>;
  view_t view(dst);
  view.center(spoint16(w/2,w/2));
  
  // create the second hand
  srect16 sr(0,0,w/16,w/2);
  sr.center_horizontal_inplace(b);
  // rotate the second hand
  view.rotation((time.tm_sec/60.0)*360);
  // here we translate each of the 
  // rectangle points using the view
  spoint16 second_points[] = {
    view.translate(spoint16(sr.x1,sr.y1)),
    view.translate(spoint16(sr.x2,sr.y1)),
    view.translate(spoint16(sr.x2,sr.y2)),
    view.translate(spoint16(sr.x1,sr.y2))
  };
  spath16 second_path(4,second_points);

  // create the minute hand
  view.rotation((time.tm_min/60.0)*360);
  spoint16 minute_points[] = {
    view.translate(spoint16(sr.x1,sr.y1)),
    view.translate(spoint16(sr.x2,sr.y1)),
    view.translate(spoint16(sr.x2,sr.y2)),
    view.translate(spoint16(sr.x1,sr.y2))
  };
  spath16 minute_path(4,minute_points);
  
  // create the hour hand
  sr.y1 += w/8; // shorter
  view.rotation((time.tm_hour/24.0)*360);
  spoint16 hour_points[] = {
    view.translate(spoint16(sr.x1,sr.y1)),
    view.translate(spoint16(sr.x2,sr.y1)),
    view.translate(spoint16(sr.x2,sr.y2)),
    view.translate(spoint16(sr.x1,sr.y2))
  };
  spath16 hour_path(4,hour_points);

  // draw them

  draw::filled_polygon(dst,minute_path,color<typename Destination::pixel_type>::black);

  draw::filled_polygon(dst,hour_path,color<typename Destination::pixel_type>::black);

  draw::filled_polygon(dst,second_path,color<typename Destination::pixel_type>::red);

}

Posted 1-May-22 23:11pm

honey the codewitch

Updated 2-May-22 1:42am

v3

Add a Solution

Comments

CPallini 2-May-22 5:39am

That wouldn't compile. You are using w before defining it.

honey the codewitch 2-May-22 5:44am

hahaha I had changed the order above after-the-fact for readability and it's early. I fixed it.

2 solutions

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

CPallini · Answer 1 · 2022-05-01T23:52:00

Solution 1

While I guess what your code is doing, I would need some documentation in order to actually use your API.
By the way Destination is not a problem for me.

Posted 1-May-22 23:52pm

CPallini

Updated 1-May-22 23:53pm

v3

Comments

honey the codewitch 2-May-22 6:35am

What I'm trying to do is create examples for people that don't read the documentation.

A lot of times people that use arduino are hobbyists and don't RTFM.

Assuming autocomplete, and the above, can figure out what it's doing from the context?

Not necessarily precisely - just imagine you're looking at a code sample to figure out how to do something. How hard is this one to figure out enough to fiddle with it, if that makes sense?

CPallini 2-May-22 7:37am

Well, the comments you gently added to the code help.
Let's see if I got a bit your code: the clock is not at center of the bounding rectangle and the hour hand is actually longer than the other ones. :-D
Do Arduino people have autocomplete (Arduino IDE does not provide it, if I remember correctly)?

honey the codewitch 2-May-22 7:42am

First, GFX doesn't work with the Arduino IDE because it won't compile with C++11. I use the platformIO IDE with it, which runs inside VS Code so you get all that autocomplete goodness, and just far more control over your project builds than you can get with the Arduino IDE.

Spoiler on the code:

Yeah, the clock basically forces the rectangle you pass it to be square by looking at the shortest edge and treating that as both the width and the height. That's what w is.

Then it *is* centered. horizontally. See, all hands are drawn effectively at the 12:00 position (before rotation), and then rotated based on the corresponding time figure.

Notice the center of the viewport is set at the beginning. That means the rotation will be around that point. =)

CPallini 2-May-22 7:50am

Still I think the hour hand is actually longer than other ones.

honey the codewitch 2-May-22 8:01am

Nope. It's shorter. I'm increasing the top y value, so it starts further down the screen, and since y2 is unchanged that makes the rectangle less tall. I swear to you it works, I'm looking at it running right now. Remember that all the rectangles are effectively anchored to the center on their bottom edge. That might be an area where I should clarify what I'm doing.

CPallini 2-May-22 8:36am

Oooops, my bad. I misinterpreted your y1 as 'second y coordinate' (my long-date C habit), while it was clearly the first one.

honey the codewitch 2-May-22 8:38am

As long as it's clear, I'm cool with it. I do weird things like that too. :)

vinclaro001 · Answer 2 · 2022-05-02T01:42:00

I can understand it fine, but then again I’ve been using C++ since Turbo C++ in DOS was a thing. I wonder if the average arduino developer would understand the concept of viewports and view coordinate systems with rotation and translation, or even C++ templates? Many seem to only know C at a first or second year level from what I have seen. Also don’t forget a lot of arduino hobbyists are in STEM courses at high school too, so they may not have a lot of experience with coding generally, let alone C++

I think examples like this are one of the only places where putting a comment at almost every line makes sense.

Of course, if your audience is an “advanced developer” then please ignore my above comments, as I think the example you’ve given is self-evident and very easy to grasp :)