Skip to content

Remove static nonsense#4

Open
henke37 wants to merge 6 commits intoBarliesque:masterfrom
henke37:patch-1
Open

Remove static nonsense#4
henke37 wants to merge 6 commits intoBarliesque:masterfrom
henke37:patch-1

Conversation

@henke37
Copy link

@henke37 henke37 commented Mar 8, 2015

Make the constructor a proper constructor, remove the getters, put the logic of the getters in the constructor.
Then remove the static keyword from all methods and properties.

henke37 added 6 commits March 8, 2015 19:09
Create a constructor, remove the getters, put the logic of the getters in the constructor.
Then remove the static keyword from all methods and properties.
@henke37
Copy link
Author

henke37 commented Mar 8, 2015

You can probably merge the Assembler class into the EasyBase class.

@Barliesque
Copy link
Owner

As soon as I get a chance I'll have a look at your amendments and probably merge it in.

While you could merge the Assembler class into the EasyBase class, I'm inclined to leave it as written by Adobe, so that updated versions can be easily dropped in without having to do a bunch of cutting and pasting of code.

Thanks both of you for your feedback!

@henke37
Copy link
Author

henke37 commented Mar 23, 2015

I meant the other Assembler class, the one actually named as such and being a part of the project.

I think the fact that I added a property named "realAssembler" is a clue that something is wrong with the design. The Assembler class just has a couple of very thin wrapper methods. It should be merged into the EasyBase class.

@Barliesque
Copy link
Owner

Ahh. That makes sense. Pardon the confusion -- I haven't had a chance yet
to actually look at the code.

On Mon, Mar 23, 2015 at 3:48 PM, henke37 notifications@github.com wrote:

I meant the other Assembler class, the one actually named as such and
being a part of the project.

I think the fact that I added a property named "realAssembler" is a clue
that something is wrong with the design. The Assembler class just has a
couple of very thin wrapper methods. It should be merged into the EasyBase
class.


Reply to this email directly or view it on GitHub
#4 (comment).

@b005t3r
Copy link
Collaborator

b005t3r commented May 27, 2015

@henke37, I'll be merging this pull request soon (can do it right now, as it breaks a couple of things I'm using right now), so if you have any comments/pointers or would like to inform me about anything - this is the right time to do it :)

@henke37
Copy link
Author

henke37 commented May 27, 2015

I have nothing specific to say. But I can give you a heads up that I wrote it here on the website and never test compiled it. I vaguely recall there being "something" I neglected to fix after my surgery too. More fun for you then!

@b005t3r
Copy link
Collaborator

b005t3r commented May 27, 2015

What? :) You mean you never even compiled it? :)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants