YAML.jl icon indicating copy to clipboard operation
YAML.jl copied to clipboard

[Not Urgent, Stylistic] Rename `length` to `len`.

Open Paalon opened this issue 1 year ago • 3 comments

Julia Base exports the name length thus rename for disambiguation.

Paalon avatar Jun 14 '24 07:06 Paalon

I read src/buffer_input.jl and src/scanner.jl many times. A few days after sending this pull request, I noticed a few things. The variable name length is used as the 2 following meanings:

  1. the position of the current peeking character (0-based)
  2. the length of the processed token

In Julia, I think this variable should mean 1-based position, and the name should be pos and len = pos - 1. The name position is also exported by Julia base thus pos will be fit. If longer name is desired, character_position may be good but I don't think the longer name is required because it is obvious that we are processing a stream and what pos or len means.

Anyway, this PR is not urgent. It's okay to think about this later.

Paalon avatar Jun 16 '24 12:06 Paalon

I don't think the longer name is required because it is obvious

I always worry about statements like this - I have on many occasions thought "this will be obvious to future me", and then come back after a couple of months it more and have no idea what's going on.

You're probably right that pos and len are obvious, but even something like charpos is a bit more explicit, and it would make me feel better :wink: .

kescobo avatar Jun 16 '24 20:06 kescobo

In Julia, I think this variable should mean 1-based position

For what it's worth, position, seek and other stream operations in Julia follow the convention to consider the position as a 0-based offset from the start of the file.

GunnarFarneback avatar Jun 17 '24 15:06 GunnarFarneback