Allow a Turntable to load a Vinyl on demand
RuiAAPeres opened this issue ยท 19 comments
Looking at this usage from a usage POV, I seem myself creating the Turntable with a given configuration and then on each test case just load a particular vinyl and use it. As it is right now, you have to create a turntable every time.
We are also considering setting the Configuration at global scale instead, so you don't have to pass one everytime you create one. cc @SandroMachado @zamith
@RuiAAPeres I think I'm missing some context here. Can you explain in more detail what you plan to do? Maybe some code samples?
@RuiAAPeres Ok, so let me see if I got this straight. You create a turntable that can have more than one vinyl and that vinyl can have multiple tracks? Or a turntable can only have one vinyl? Or the the vinyl can only have one track?
I couldn't really figure this out from the README, sorry.
@RuiAAPeres Doing some source diving it seems like a turntable can only have one vinyl. Which leads me to the question, why do you need a turntable at all?
@RuiAAPeres Doing some source diving it seems like a turntable can only have one vinyl. Which leads me to the question, why do you need a turntable at all?
@zamith although that is currently true, it gives us more space to think about other features, like adding multiple Vinyl
to a single Turntable
.
@RuiAAPeres From my experience with something like VCR, there are only two concepts, the cassette or track, which is the actual data, and something that holds the configs, the VCR or what would be the vinyl. I'm unsure of why you would need an extra layer.
You might argue that you want/need to have different configurations in the same app, which I'm not sure is very common. But in that case, you could solve it by having the configs be set on an instance of something and not at the class level, which would make changing the configs a matter of changing the instance you use on a particular test or set of tests.
What things can you foresee as being interesting to add to a turntable? Am I missing something here?
@zamith for instance, two integrations (service A and service B) on the same application. A turntable
, two Vinyls
(one for the integration with the service A and another one for the service B) and a set of tracks
for each Vinyl
.
What do you gain from having two vinyls? You could just as well make the tracks have names, and one vinyl with the tracks for all of the services.
But yeah, if you feel like you need to have different data structures for such a situation, it might make sense. It would definitely be good to have a global config in this situation, though.
IMO, it is much more easier/cleaner to have two different vinyls
for each integration. Or for each test case...
@zamith, Turntable
is the mechanism that we use to intercept all network traffic that goes through NSURLSession
. This interception could be performed using other methods but we definitely need some layer to execute it and perform the corresponding mapping.
I don't think that conceptually a Vinyl should be concerned in doing that, a Vinyl is nothing more than a container of tracks which are the ones that really contain the data. You cannot play that black plastic without a proper device that understands it and can translate his information into music. And the Turntable
is that device in our case (actually there's also an internal player involved).
I agree that a turntable should not be restricted to a single vinyl otherwise it may seem redundant, but I really believe that this layer is beneficial to achieve a good separation of concerns.
@dmcrodrigues Yeah, I get that. Maybe it's the current implementation that led me to question what's the benefit of Vinyl
over [Track]
.
When you go out and want to take your laptop, mouse and headphones normally you take a bag to carry them over! ๐
Basically its more manageable having a Vinyl
instead of working directly with [Track]
, e.g. parsing from disk, but yeah it's only a container.
I think we diverged a bit on this issue, so lets back on track (pum not intended).
So, currently we need to create a new Turntable
in order to load a Vinyl
. There are two ideas on the table:
- Allow a
Turntable
, once it's created, to load multiple times aVinyl
. - Allow a Global configuration to be setup, so we wouldn't have to replicate a lot of boilerplate every time a new
Turntable
is created.
These two things might seem orthogonal, but they are related to the same thing: ease of use. When you have a XCTestCase
we have the following scenario:
class FooTests: XCTestCase {
func test_1() {
// Create Turntable configuration
// Create turntable with Vinyl + configuration
// Use the turntable
}
func test_2() {
// Create Turntable configuration
// Create turntable with Vinyl + configuration
// Use the turntable
}
}
Looking at how the lib is working right now, the user could just move the configuration to the XCTestCase
scope
class FooTests: XCTestCase {
// Create Turntable configuration
func test_1() {
// Create turntable with Vinyl + configuration
// Use the turntable
}
func test_2() {
// Create turntable with Vinyl + configuration
// Use the turntable
}
}
With the 1st proposal (Turntable
allowing multiple Vinyl
loads) :
class FooTests: XCTestCase {
// Create turntable with Configuration
func test_1() {
// load Vinyl
// Use the turntable
}
func test_2() {
// load Vinyl
// Use the turntable
}
}
With the 2nd proposal (Global Configuration #33) :
class FooTests: XCTestCase {
override func setUp() {
// Setup Global Configuration
}
func test_1() {
// Create turntable with Vinyl
// Use the turntable
}
func test_2() {
// Create turntable with Vinyl
// Use the turntable
}
}
This 2nd proposal is for me the worst, since we have to override the setUp
func (see note). The problem I have is not the overriding intrinsically, but that method setting up the Global configuration every time before each test is run.
There is also a 3rd possibility which is implement both the 1st and 2nd proposal, which I am against, since we will just clutter the API and make the Turntable
implementation more complex.
Bottom line, I'm in favour of:
- 1st approach (
Turntable
loading multipleVinyl
).
Note: It could be another method tearDown
or when the XCTestCase
is initialized (somewhere else?), the point is: it becomes more cumbersome for the user.
I agree, the 1st approach seems more logical and less cumbersome for the user.
๐
I agree with the first approach as well. However, is there a setup method
that runs only once per suite? That way you could create the default
turntable there and if need be create a different one (different configs)
in a particular test.
On Sat, 20 Feb 2016 at 10:08, David Rodrigues notifications@github.com
wrote:
I agree, the 1st approach seems more logical and less cumbersome for
the user.๐
โ
Reply to this email directly or view it on GitHub
#31 (comment).
@zamith yes, XCTestCase
has two methods for setup:
class func setUp()
- Runs once per suitefunc setUp()
- Runs before any test
+1 for the 1st approach.