Implementing circle

[thomasguillard]

This is adding a new method to plot circle on the screen. It is called using screen.circle() and requires a position. You can also give it a radius and a color.

This will let users have access to a new shape, ideal for projects using balls, planets, coins (and everything round really)

[joerick]

Hi @thomasguillard - this is great. Thank you.

I've been thinking about the API for this... imagine we also have an oval() function (we will one day). What will be API for that be?

For consistency’s sake, I think parameters should match the rectangle function, that is

def oval(self, xy=None, size=(100, 100), color='grey', align='center'):

So I think we should make sure that the circle function is consistent with this function. I note that the above code (where width and height are equal) would actually draw a circle! So how about this for the circle API:

def circle(self, xy=None, size=100, color='grey', align='center')

then we have the advantage that the APIs are all consistent.

An added advantage of this is that the circle function can call the oval function. I’d actually argue that they should be the same function, just with a different name/documentation - and oval() can take a number or a 2-tuple - there’s no point in the library being pedantic.

What do you think?

[thomasguillard]

Hi @joerick ! This is a very good point, I didn't think of that. This is also making sense for being consistent with another one we will consider later I'm sure which is the arc shape. Pygame doc says: arc(Surface, color, Rect, start_angle, stop_angle, width=1) This is also based on using a rectangle to draw a specific ellipse section so that should also follow this consistency.

I really like your idea, the circle function should definitively just be a special case of the oval function.

I'll work on that!

I can also see that you want to get the align argument in this function too. I left it aside and actually forgot to include it at the end, I will look into it as well.

Thanks for your comment and for sharing your opinion!

[thomasguillard]

Hi @joerick , I have been adding a bunch of checks to xy and size but another idea came to my mind doing so.

If we want some consistency for the API calls, wouldn't it be better to perform checks for xy and size in all the screen methods?

If so, it would be more efficient to create checking functions for xy and size separatly and calls them in each screen functions.

What do you think?

[joerick]

Hey thomasguillard, sorry for the delay in getting back to you. I think there is a bigger job improving parameter checking throughout the library but let's stay focused for this PR and just worry about circle.

Just a few notes on style

1. could you try and write the code so that parameter checking comes at the start of the methods and isn't getting muddled into the actual drawing code? As a rule of thumb, try to only have one line that's doing the actual 'work' of the method and have that at the end
2. A lot of the comments in this code are redundant- you're just saying what each line does, but that should be obvious by reading the code. Could they be stripped out unless they're saying something that the code isn't saying?

[thomasguillard]

Hi @joerick !

I'll look into it! ;-)

[thomasguillard]

All (daft) comments have been taken off. Don't know what I was thinking about...

I don't get your request about having one line doing the drawing, since we introduced checking I need two different cases for each method.

One if you send a number to circle, and another if you send a list to grab the first element. One if you send a 2-tuple to oval, and another if you send a list to grab the two first elements?

Or am I missing something?