Re: [PATCH v12 2/2] rust: xarray: Add an abstraction for XArray

Miguel Ojeda
2024-12-13T20:03:51Z

On Fri, Dec 13, 2024 at 6:55 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Implicit in this statement is the assumption that while new to Rust,
> those reviewers will not be new to C.

Sorry, I am not sure what you mean.

I was talking about maintainers and key reviewers that need to read
your code, and thus the "advantageous for the submitter" bit, who most
likely are able to read C.

But even if you are talking about a reviewer that is new to C (or even
both C and Rust), they will be able to understand your Rust code if
you are writing an `if` instead of `.then()`.

It is not about what style is best, but about making it readable for
the majority of readers.

> Anyway, I'm not emotionally attached to this style. Would you prefer I
> rewrite it using loops and conditionals? Whatever helps this land, I
> will do.

Personally, I would rewrite at least the `if` one, since it is not
really how we write those elsewhere. The other one, since you are
building an iterator anyway, I think it makes sense. For similar
reasons, perhaps one `match` you have could be an early return
instead.

But it is up to the maintainer. By the way, I don't see the XARRAY
maintainer nor the list Cc'd?

Thanks!

Cheers,
Miguel