Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Class instance variables? #91

Open
jimfoltz opened this issue Sep 27, 2013 · 5 comments
Open

Class instance variables? #91

jimfoltz opened this issue Sep 27, 2013 · 5 comments

Comments

@jimfoltz
Copy link
Contributor

I'm not sure I like how I use class instance variables in some of the methods. For example, the @mesh_file variable which shows up in several methods. It's used more or less like a global variable, and is in fact "global" to the Exporter scope. The way it just pops up in methods is a little jarring. The more I look at it, the less I like it.

So what is a better way to write the method? Should I add a file argument to each of the methods that use @mesh_file?

@thomthom
Copy link
Member

Yea, I think that might be useful. I think that the less instance variables the methods use the easier they are to unit test as well. I've also found it generally easier to follow the code flow with less instance variables.

@briangbrown
Copy link
Contributor

You can always just add a get_file method to provide yourself a seam for testing if you don't want to add a file param to each method. It is a bit weird that the methods that write to @mesh_file assume it is in a certain state and even close it as a byproduct of writing the footer. That said, this isn't Java, so you can always change @mesh_file to anything at any point in a test.

I do like the way you setup the @write_face method. It would be nice to do the same for write_header and write_footer. It would make adding a new format easy (though there may never be another).

@thomthom
Copy link
Member

We do have another project started, PLY. We're reusing quite a bit of the structure from STL.

There have also been talks about AMF format in the issues. The more flexible and reusable we can make the importer, exporter project the better it would be for future projects. We could in fact create a boiler plate template. Something that would be very nice once we get test units up and running.

@thomthom
Copy link
Member

thomthom commented Oct 9, 2013

Is this still valid?

@jimfoltz
Copy link
Contributor Author

jimfoltz commented Oct 9, 2013

The write_face method is still being stored in a @write_face variable, so still open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants