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

AbstractFactory Confusing methods #847

Closed
markbolster12 opened this issue Feb 8, 2019 · 4 comments
Closed

AbstractFactory Confusing methods #847

markbolster12 opened this issue Feb 8, 2019 · 4 comments

Comments

@markbolster12
Copy link

In the App class of the abstract-factory pattern, there are several methods which seemingly only serve to confuse readers.
Is there a point to having a method that returns a King, from a passed in KingdomFactory?

King getKing(final KingdomFactory factory) {
     return factory.createKing();
   }
@iluwatar
Copy link
Owner

Apparently the method you mentioned is used in the tests. If you have a vision how to make the example clearer I would be glad to accept the pull request.

@markbolster12
Copy link
Author

markbolster12 commented Feb 25, 2019

One way I think that would make sense, is to alter the test. The test would instead of creating 2 factories and then testing those factories with a method never actually used (and which isn't obviously useful), the test would create the 2 factories, and then create 2 versions of App, calling createKingdom for each factory, and doing the tests like
assertTrue(app1.getKing() instanceof ElfKing);

One other option would be to use a singular app instance and do all the tests for a particular kingdom first, then call createKingdom for the other type. Would appreciate input as to which would be more clear. I'd be happy to update it either way, and then remove the static methods from the app class.

@iluwatar
Copy link
Owner

I'd vote for the second option - sounds very clear

@Blackgard
Copy link

До и ш

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