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

Etl: Do we really need an instance of the class Etl? #90

Closed
NobbZ opened this issue Jan 23, 2016 · 1 comment
Closed

Etl: Do we really need an instance of the class Etl? #90

NobbZ opened this issue Jan 23, 2016 · 1 comment

Comments

@NobbZ
Copy link
Member

NobbZ commented Jan 23, 2016

In the exercise etl, the current testsuite does enforce a non-static class by creating an instance of Etl.

When I did the exercise about half a year ago, this was not the case.

Since there is no data passed to the constructor and you can leave everything it is and you don't need to implement a constructor yourself, but let it fall through to Objects constructor, there is really no need to enforce instantiability.

So why was this change done? Why don't the current tests use a static Etl?

@jtigger
Copy link
Contributor

jtigger commented Jan 24, 2016

Well, this is a good question, @NobbZ. Thanks for inquiring.

Looking back in the git history

git log --stat -- **/EtlTest.java

the earliest version I see of this test is @e53508827ad3b7b0691abe2cde2fcdd5a525562b. At that time @sit was using the instance method style. So, I'm not able to compare it with a prior style.

This exercise does not require transform() to be either a class member or an instance member — either solution "works." So, any conclusion we might draw here won't be some "one true & right" answer.

That said, static keyword means "this member is shared with all instances of this class." I infer that, "it would be inappropriate to send this message to an object; it's utility code." In this case, though, transform() contains a reasonable amount of "business logic." Given this, seems like the instance member move is the "better" of the two.

Now, it's true that we might likely want Etl to be a singleton; but that is expressed in a different way: either natively or by use of a dependency injection container.

Thoughts?

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

No branches or pull requests

2 participants