search

jeanluct / braidlab

Matlab package for analyzing data using braids
GNU General Public License v3.0
23 stars 9 forks source link

Does getgraph need all those globals? #62

Closed jeanluct closed 9 years ago

jeanluct commented 9 years ago

In the function body Marko states "Globals are used to speed up computation in local hash functions" where "hash function" refers to private/graph_hashtokey.m and private/graph_keytohash.m. Is this really crucial?

jeanluct commented 9 years ago

From Marko Budisic on 2014-09-23 20:31:50+00:00

No, I don't think so. That whole part was written in haste and is poorly documented as well. I think a more accurate statement is that the globals were used to avoid a long chain of passing top-level variables to low-level functions. I've looked through MATLAB support forums and it seems that Matlab passes by-value if there is in-function modification of an object, and by-reference if there isn't.

Since, M, N and T are only queried later, I don't think there's a reason not to pass them as argument, aside from the bloat in the function call.

jeanluct commented 9 years ago

The bloat can be mitigated by using a struct: curve.M, curve.N, curve.T. Then pass only the struct.

jeanluct commented 9 years ago

In any case, this isn't very important for now. Feel free to drop the 2.2 milestone, but we should fix it eventually.

jeanluct commented 9 years ago

Got rid of the globals by packaging them in a struct and passing to the function.