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

We need to explicitly define the object lifecycle #401

Open
Silverwolf90 opened this issue Jul 23, 2016 · 2 comments
Open

We need to explicitly define the object lifecycle #401

Silverwolf90 opened this issue Jul 23, 2016 · 2 comments

Comments

@Silverwolf90
Copy link
Contributor

Silverwolf90 commented Jul 23, 2016

Two major problems:

  • VexFlow has no clearly defined object lifecycle
  • VexFlow provides no support in assembling objects the right way

This is something that that tests get really wrong. They all construct, setup, and draw things in different order. It's incredibly inconsistent. We're always telling people to look at the tests to learn how to use VexFlow's more obscure parts, but as I'm looking through them now. It's chaos. We can't expect people to look at the tests to get any clarity on how to use this library.

I'm bringing this up because I was trying to make a very reasonable change to StaveNote:

Currently, a StaveNote sets the x value for it's Noteheads and Stem in it's render() method. But this is clearly the wrong place. We shouldn't be mutating anything in render(). So I moved that logic into .postFormat(). Since that's clearly the the place to set the x values. However, doing so broke tons of tests because lots of them are doing things in the wrong order. But that fact would have never been apparent when the tests were written.

@Silverwolf90
Copy link
Contributor Author

Another related question: when should Glyph's be initialized? I think, ideally, all Glyph based Elements would construct their glyphs at the same point in the Element lifecycle.

Provided that Elements have methods used for configuration after construction, I think they should be initialized right before formatting (ie: after the object has been been fully configured).

@0xfe
Copy link
Owner

0xfe commented Sep 4, 2016

Right now I think there's too much initialization in the constructor, causing a lot of issues with configuration after construction. I think as much as possible should be pushed out to the preformatting stage.

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

2 participants