
In metapict
I have rotate
whose angle argument is in radian. The function rotatedd
uses degrees for the angle argument.
Now rotate-about
rotates some angle (in radian) around a point. Should the degrees version be named rotate-aboutd
or rotatedd-about
?

+1 for rotatedd-about

(writing documentation on transformations)

could you make wrappers like (degrees 270)
and (radians pi)
? then you don’t even need multiple functions to accept both

the extra d
naming convention is obscure and easy to misread, and I don’t think there’s anything that suggests it’s obvious that the radians are the default

@notjack’s idea sounds like a good idea to me

you could use the measures
package :stuck_out_tongue:

There are already conversion functions back and forth between radians and degrees. The “problem” is that mathematically the sensible default is radian - but when drawing everybody thinks in degrees.

No I mean make structs for radians and degrees

Ahh. Need to think about that.

(struct radians (value) #:transparent)
(struct degrees (value) #:transparent)

It’s tricky to make convenient. Need special versions of sin, cos, and friends.

Use a parameter?

I’d rather read a few extra (degrees ...)
and (radians ...)
wrappers than read duplicated versions of each function that takes an angle as an input

MetaPict is leaning towards the terse end of the spectrum. It’s due to the inspiration from MetaPost and Tikz, which both are very terse.

The d-convention is already in use in several places (sind, cosd, pt@d, etc) so I think it is too late to change.

But I get that a “value with a unit” is better represented as a struct.

Keep them, add a parameter, make radian the default, and make radian function work with degrees based on parameter

And Mark the d variants as deprecated

I’d actually rather use separate functions than a parameter

Why?

Poor style - but I have to go. I’ll be back later.

if it would be weird for someone to pass in a non-literal for an input, then don’t give them the option. Parameter means you could do something like (parameterize ([angle-mode (if tuesday? 'degrees 'radians)]) ...)
, which doesn’t seem like a use case that should be encouraged.

That sounds very contrived to me

I don’t doubt you have a good reason, but you’ll have to find a better counter example to convince me :)

(because that’s true for every parameter ever, like log-ticks and whatnot)

One sec, I’ll dig up the design guidance from work that I’m half-remembering

So the specific guidance is “use two uniquely named methods instead of a single method with a boolean parameter” (in a Java context). > If the parameter is used to change some aspect of the method’s behavior, consider providing two differently-named methods instead. A common prefix is probably a good idea. For example, see FluentIterable’s partition
and partitionWithPadding
methods. Note that we can easily count or view the usages of these two methods separately. > > Different behaviors call for different methods. Of course, these methods can still be _implemented_ by passing a boolean
to a private
helper. > > If this is a constructor, you can’t exactly split it into differently-named constructors. But you can switch to factory methods, as recommended in _Effective Java_ <https://goto.google.com/ej3e–1|Item 1>. > This change may result in some callers having to write code such as condition() ? foo.methodA() : foo.methodB()
. If it’s _many_ callers, you may need to reevaluate. In such cases prefer the alternatives below (@notjack which I’ve omitted from this slack message) instead of a boolean. > > (Aside: this same advice holds for methods that use null or a special magic value to trigger an alternative behavior. Provide for that behavior explicitly, with its own method, instead.) I think a similar spirit applies to dynamic parameter v.s. separate functions. Really I’d rather not do either of those, I’d prefer wrapper types so the function can act like it’s overloaded, but if that’s not feasible than duplicate functions would be my second choice.

the reasoning isn’t mentioned much on this page, I’ll see if I can find some of the discussions that led to this advice

I don’t think this applies here. Its not different behaviours, is different units. If that applied, then the same argument would apply to all arguments, and all functions would have no argument

The problem with the parameter is that it’s silent. You may not know if you’re currently using radians or degrees

But it’s usually not true because everyone should leave the default value of the parameter when exposed to the user

I don’t see how it would apply to all arguments. Most arguments don’t come in multiple flavors of units.

The guide resembles the style used in Apple apis (which has very long names - but they are very easy to google). If we consider keywords part of the name of a function, then (rotate #:radian 3.14)
(rotate #:degrees 180)
are more or less the same as rotateWithRadian
and rotateWithDegrees
.

So the rationale must be: If a “small” change in the arguments triggers a “large” change in behaviour, then it can potentially lead to bugs. Therefore we advertise the difference using different function names.

It’s also about call-site readability

Nice term.

strongly agreed (and not my invention)

All arguments change the behaviour of the function

I’m not sure about the parameter yet, one way or the other, but i don’t find duplicating every function particularly tasty.

I don’t like (rotate #:radian 3.14)

It allows people to write (rotate #:radian 3.14 #:degrees 10)

so you would need extra check to make sure that they are mutually exclusive

The conversion functions to put everything in radians don’t seem so bad to me

The mathematician in me likes the “stick to radian” approach. But the d-functions are (hopefully) convenient for people that only know about degrees (and haven’t heard about radians).

Then you’d have to give them a proper name like rotated/degrees, and for those people the rotated one will be confusing

Syntax parse has this, we should have this for functions too.

I feel really conflicted about this

syntax-parse
is OK because it operates at compile-time.

For functions, you would want your functions to run fast at runtime (especially for recursive functions — it gets invoked many many times)

So I dislike runtime check for mutually exclusion

Compile-time check is possible, but you need to change define
/lambda
, which is not appealing

Fair enough. One more thing on the list for racket2?

(lambda (x) x)
?

changes the behaviour of the function: the return value is different if the input value is different.

What I mean is that I don’t see a clear separation line

I think it’s different for boolean parameters because you can actually reasonably make a function for each possible input.

and when the boolean degree?
turns into a category unit: (one-of 'rad 'deg 'grad)
? or if you have 2 booleans? The ‘call site readability’ is a good argument, and keywords do substantially mitigate the issue (by contrast to Java which has only positional arguments, unless this has changed)

but the description doesn’t hint at that argument.

This leads to an interesting general question: The good default is A, but the majority knows only B. I can make B an option, but then those who know only B will be confused by A. Those who know A will be annoyed/irritated if B is the default. How do we solve this?

Also, keyword calls are notoriously ‘slow’. Don’t use keywords on recursive functions. (Though most of the time I start with a recursive function and end up using an internal let loop
to avoid passing unchanging arguments around)

Can the unit be included? Eg (rotate n degrees)
or (rotate n radians)

~~~That way you annoy everyone~~~

For context, it’s from the internal java best practices site, which tries to provide terse guidelines (so mostly without long explanations). For minor stuff the reasoning behind the guidelines is usually not on that site, it’s found in separate mailing list discussions and bug reports that motivated the change to the site.

@spdegabrielle that’s about what the wrapper types do, just with some extra nesting

> The good default is A, but the majority knows only B. If this is the case, then I think it can’t be assumed that the good default is A. The API needs to put in effort to explain that to the reader and convince them of it.

Suppose good = only option used by experts.

Do you want to require that users of your API be experts?

(not necessarily a bad thing to do, but definitely a choice that should be made consciously)

I thought they were roughly compiled away in most cases

unrelated: hi everyone, I’m so glad this channel exists :smile:

I think it’s not so much that the option is only known by experts, but that once you know enough to be an expert, you know that it’s definitely the right option

If this is in speed-critical code, specialized functions are probably good to have around.
The specialized functions should probably have the unit in their names, even if the unit is radians. People who are immersed in degrees or radians might not even consider there’s a choice unless they’re required to make it explicit.
If the extra verbosity is concerning, then putting the unit in the name of the module might be a way to keep the choice explicit but keep programs terse at the same time. However, even this might be a recipe for some surprises; when someone’s debugging a piece of control flow that passes from one module to another, they might not realize that one module uses radians and the other uses degrees. It’s a hazard to watch out for, and in this case I think I’d just live with the verbosity rather than risking it.
For things that aren’t so speed-critical, a good abstraction would be to have an angle data type like notjack’s saying, with at least degrees
and radians
constructors. That way, we don’t lose any precision, the intended units of the angle are clear, and the units are easy to forward along from one function call to another.
The specialized versions and angle data type can coexist.

Oh the performance is a good point

(someone please come up with a way for me to define functions that include simplification/inlining rules to optimize away allocations like that)

pie-in-the-sky approach: (rewrite-rule rotate-degrees-specialization
@summary{Using the @racket[rotate-degrees] function avoids allocating the intermediate @racket[degrees] value.}
#:surface-as hidden-optimization
(rotate (degrees 270))
(rotate-degrees 270))

starting with something like that, I’d find it nifty to be able to write (define (rotate-forwards
x) (rotate x))
and have it automatically get the same optimizations

I think I wouldn’t use that, probably

or rather, wouldn’t need that

if I’m making a new API on top of an existing one, I’m not sure there’s many cases where I want to wholesale copy a ton of rules

preserved this in a gist: https://gist.github.com/jackfirth/7f29e2e79d37a37e9a3c7059da5da594

Keyword calls are slow when they can’t be inlined. Unless you pass a function that has keywords around as a higher-order value, inliner is likely to succeed.

@notjack can you submit this at https://github.com/racket/rhombus-brainstorming?

Sure, I’ll try to get around to that soon.

How about specifying the type of the angle as a symbol? (rotate 270 'radian)
, then we are back to the A is better but expert and B is more commonly known issue on what do choose as default to keep most of it terse

There are similar apis in the lib

why is that better than (rotate (radian 270))
?

the default could be 'radian
, which seems to be the case of the lib anyways. So you’ll specify (rotate 360 'degree)
where you want degrees… But the main point is that it’s present in the standard library already for some operation (just that I can’t find it right now…)

it’s halfway verbose I’d say

What I like about using binding is that if you misspell, it’s a compile-time error. If you misspell a symbol, you might not even know it if the code is not reach in your test

Also, you can carry around the information

yeah I like compile-time errors the best but I noticed this pattern in the core library and I saw it missing in the discussion so I thought it was worth pointing it out

I think it’s a pattern in the core library because it’s easy and simple for the implementor, not because it’s ideal for the caller