Avoid reducing code readability

[HalfVoxel]

I see this change was made quite recently. In my opinion it makes it significantly harder to understand that bar calculates barycentric coordinates, compared to a method named Barycentric.

[andywiecko]

It is a matter of preference, I guess. For me, bar is a sufficient name (and even better when it is used in mathematical context) since this is a private, non-public method.

This bar is only used in PointInsideTriangle and probably could be converted to a local function.

[andywiecko]

@HalfVoxel PS bar is a common name used in the literature for this 🙂

[andywiecko]

@HalfVoxel

I've been thinking about your issue.

In my opinion, many programmers tend to overcomplicate names for functions, variables, etc. This becomes a real problem when dealing with code that heavily contains mathematical expressions. I do not see a problem with using short names for math or internal purposes. I believe shorter names are even better, but everything should be balanced, of course. There are many other names in the project which have some ambiguous meanings, e.g., Kron, max, min, div, etc. Potential contributors/developers should learn about the usage of the internals themselves. Longer or shorter name will not help. In your example, I think shortening Barycentric to b may be too short (but not necessarily, e.g., in a local function this should be fine), but Barycentric to bar seems very reasonable.

To partly resolve this issue, I modified the CONTRIBUTING.md file

+ **Avoid overly complex names.** Simple names are usually more readable and easier to understand, especially in mathematical expressions and for internal use.

I hope this helps.

Best, Andrzej

[HalfVoxel]

I respectfully disagree.

I'm also not a fan of too much abstraction and names like BeanManagerFactoryFactory. But I also think names should be clear. It's all well and good if you are working on a project alone, but if you want contributors to join in, then having comments and understandable names will help so much.

Case in point: I am pretty familiar with barycentric coordinates, I even wrote a method using them only two days ago. Yet, it took a lot of head scratching for me to realize that bar meant barycentric coordinates. Even still, without more context, I wouldn't have known if this method calculated barycentric coordinates from a point, or if it used barycentric coordinates to get a world-space point in a triangle.

max, min, div have pretty well understood meanings, even for people who are not mathematicians and programmers. kron is a bit sketchy, but at least googling for "kron function" immediately gives you the right result. Googling bar function gives you absolutely nothing relevant.

Having bar as a variable name would be totally fine, I think. But that is predicated on being able to see where that value came from (perhaps a method named Barycentric), so that one can quickly understand what it means.

Remember, mathematical literature is typically self-contained and they define their variables. If I read a paper that used the symbol ξ, I would expect to be able to find the definition pretty quickly. That is to say, if one must have extremely short names, then there should at least be comments explaining what it means. For example, I have absolutely no idea what the alpha function you have means:

public readonly double alpha(double D, double d, bool initial) {
    var k = (int)math.round(math.log2(0.5f * d / D));
    var alpha = D / d * (1 << k);
    return initial ? alpha : 1 - alpha;
}

All names are just a single letter (alpha, D, d, k), and it's pretty much impossible to figure out what it actually does without perhaps trying to find the specific paper you used as a reference for implementing this algorithm (which I also believe is not written anywhere).

You may write your library in any way you like, but I'm just telling you that for me as a contributor, it would be a lot easier if names were easier to understand.

[andywiecko]

@HalfVoxel In my opinion, your points don't make sense and seem to be influenced by the clean code principles, which I personally dislike and disagree with. To contribute meaningfully in this area, it's important to know what you're doing. Coding alone isn't enough—you need to understand the relevant literature and commonly used notations.

For example, how is ConcentricShellsAlphaParameter better than just alpha? To grasp this, you need to study the papers and research behind it. I've provided references to the literature in the documentation. As for commenting on internals, it often doesn't make sense since it introduces additional clutter and text that needs to be maintained. The idea that code should be 'self-commenting' is, in my opinion, a myth and a bad practice.

Additionally, I'm unsure about the purpose of opening issues like 'Avoid reducing code readability.' Project issues are typically focused on the API, bugs, performance, and features. Reporting on naming conventions seems unnecessary and a bit overly meticulous.

Best, Andrzej

[HalfVoxel]

In my opinion, your points don't make sense and seem to be influenced by the clean code principles, which I personally dislike and disagree with.

I haven't personally read these, so I can't comment on them.

To contribute meaningfully in this area, it's important to know what you're doing. Coding alone isn't enough—you need to understand the relevant literature and commonly used notations.

Sure. But for a new contributor to a package, having good names/comments can make it significantly easier for them to get to that level of understanding.

For example, how is ConcentricShellsAlphaParameter better than just alpha The first one does indicate that it has something to do with concentric shells. But it also contains "alpha" and "parameter" which are pretty redundant. Perhaps I could suggest edgeSplitFraction, as this indicates what it is used for.

/// See `Delaunay Refinement Algorithm for Quality 2-Dimensional Mesh Generation` page 40.
float edgeSplitFraction(float referenceConcentricShellRadius, float edgeLength, bool initialSplit)

I've provided references to the literature in the documentation.

Indeed, you have. Looks like you've added the documentation page since I last checked out this repo. Looks nice.

The idea that code should be 'self-commenting' is, in my opinion, a myth and a bad practice.

I agree. But it's either self-commenting or comments. The absence of both is what makes code hard to understand.

As for commenting on internals, it often doesn't make sense since it introduces additional clutter and text that needs to be maintained

True, to some extent. It's a balance that depends on team size primarily.

Additionally, I'm unsure about the purpose of opening issues like 'Avoid reducing code readability.' Project issues are typically focused on the API, bugs, performance, and features. Reporting on naming conventions seems unnecessary and a bit overly meticulous.

Sure. I didn't expect this to start a debate. Sorry about that. I just thought that the diff I mentioned in the first post was such an obvious case of making things harder for contributors, that I thought I'd mention it. Obviously we disagree.