stainless-steel / dft

Discrete Fourier transform
Other
25 stars 8 forks source link

Use Box<_> instead of Vec<_> for 'factors'. #9

Closed khrs closed 7 years ago

khrs commented 7 years ago

We are not allowed to modify it after creation anyway. So let's enforce it.

IvanUkhov commented 7 years ago

Can you elaborate on why it is needed? What does it bring to the table?

khrs commented 7 years ago

Just convention. We shouldn't "push" new elements to "factors" after Plan::new() finish So we can enforce it using Box<_> Just cosmetics.

IvanUkhov commented 7 years ago

How can one push new elements into a plan?

khrs commented 7 years ago

You cannot from outside code. As I said just cosmetics.

IvanUkhov commented 7 years ago

This change makes the code less intelligible without a good reason.

khrs commented 7 years ago

I don't agree here. You use Vec<_> when you want "dynamic sized array" If you algorithm build "factors" in Plan::new() then you wanted to add/remove some "factors" later in your algo Vec<_> is fine. But in this case you build it once in Plan::new() and you never should add/remove elements from "factors" later in your algo. So we can enforce that you never add/remove smth accidentally from "factors" using Box<_> instead of Vec<_>. But it's just cosmetics.