DATAGRAPH-175: Improve HelloWorlds example

[agibalov]

Hello Michael,

Please review my changes for DATAGRAPH-175 - "Improve HelloWorlds example". Things I did:

1. Removed all unit tests from both hello-worlds and hello-worlds-aspects, since I believe there's no sense to have them in the application that aimed just to say "hello world".
2. Removed index by moons and other things of the same kind. Both applications are now distilled: no features like cypher and minimal magic.
3. The scenario implemented in both apps is: populate the galaxy, find Earth by name, get connected worlds, get all worlds.

I'm slightly concerned about having cloned code in both applications - differences are really minor, so the most of the code is just the same. Please, let me know what you think.

[jexp]

Perhaps use repository.save(world) or template.save(world) instead of world.persist()

Also I'm not sure about removing all the tests, b/c they show how you'd set up a test in a SDN scenario which I think is also important, so perhaps at least keeping one simple test for the MyWorldRepository would be sensible?

I'm also not sure if we should keep the MyWorldRepository (which is rather a service), or just use the plain graph repository and create the worlds as part of the main app.

WDYT ?

Thanks

Michael

[agibalov]

Perhaps use repository.save(world) or template.save(world) instead of world.persist()

That's a good idea. In this case the only difference between aspects and plain version would be the World fields annotations. So, are we still going to have 2 totally separate projects? On one hand that makes sense because "hello world" should be as simple as possible and having a hello world of 2 maven modules seems really weird.

On the other hand, we'll have to maintain both, just to demonstrate that there are 2 approaches and both provide almost the same with minor difference in the code.

Regarding tests. What if we just implement a GalaxyService (@service) that provides a POJO interface (WorldDTO instead of World) and internally uses SDN? In this case:

1. We're getting rid of MyWorldRepository (I do agree, all these xxxImpl and MyXxxx are embarrassing)
2. We'll be able to write some straightforward integration tests for GalaxyService

Does this sound like a plan?

[jexp]

Sounds good.

We could have one pom with a secondary profile for the aspexts dependency.

Michael

Sent from mobile device

Am 16.05.2012 um 09:26 schrieb loki2302 reply@reply.github.com:

Perhaps use repository.save(world) or template.save(world) instead of world.persist()
That's a good idea. In this case the only difference between aspects and plain version would be the World fields annotations. So, are we still going to have 2 totally separate projects? On one hand that makes sense because "hello world" should be as simple as possible and having a hello world of 2 maven modules seems really weird.

On the other hand, we'll have to maintain both, just to demonstrate that there are 2 approaches and both provide almost the same with minor difference in the code.

Regarding tests. What if we just implement a GalaxyService (@service) that provides a POJO interface (WorldDTO instead of World) and internally uses SDN? In this case:

1. We're getting rid of MyWorldRepository (I do agree, all these xxxImpl and MyXxxx are embarrassing)
2. We'll be able to write some straightforward integration tests for GalaxyService

Does this sound like a plan?

---

Reply to this email directly or view it on GitHub:
#47 (comment)

[agibalov]

Thank you Michael - will do it later today.

[jexp]

Hmm I don't think the DTO and Mapping add simplicity to the example, I'd rather have it straightforward and minimal.

[agibalov]

That's not a final version. I'm away on business, so there are no changes so far.

[jexp]

ok cool :)

[agibalov]

Hello Michael,

My 2 primary points here are:

1. We should have a "service" and in tests we should only work with this service, we should not access SDN things like repositories, node-backed entities, etc.
2. Service should never expose its internals, it just should be impossible for user to tell whether there's SDN/Neo4j or Hibernate/HSQL behind.

Point 1 is important because otherwise we'll only be able to test SDN things. That's a nonsense because there's a bunch of tests in SDN project and writing these tests once again just doesn't make any sense. On the other hand, writing tests for the service means we "state" that there's a couple of scenarios implemented in the service and we just show what these scenarios are.

Point 2 is pretty close to point 1 - just if we expose SDN entities to the user, it's a kind of SDN testing, so much more interesting is testing what we exactly implemented in the demo.

We could probably use detached SDN entities instead of DTOs. Not sure if there's a real profit, but at least we won't have mapper and DTO. Just less code. Could you please clarify your expectations regarding the code?

Thanks,
Andrey A.

[jexp]

Sorry I don't get it.

The DTO conversion is only useful if you want to serialize the data in a certain way or provide it to some other layer that doesn't know and doesn't care about the domain (e.g. ui-views or some processing). In this case where it is about the domain, why move away from the domain classes?

This example should be about the mapping to the graph and usage of template and repositories, everything else will imho just confuse the reader.

[agibalov]

OK. In this case I'm just removing this extra mapping layer and use the entity classes as is. Thanks! Will commit later today.

[lassewesth]

Hey Andrey,

I have a couple of comments.

1. I don't like removing tests. Use them as the driver for the example. Instead of printing stuff to console, use the test to illustrate what I am supposed to see from this example.
2. In the interest of minimalism, I don't like WorldDto and the Service. We want to showcase super-simple SDN, so I think a simple WorldRepository implementing GraphRepository is fine. It illustrates the minimal stuff you need to get going with SDN.

Keep the advanced concept for other tutorials ;)

Lasse

[agibalov]

Hi Lasse,

1. Not sure what you mean - there are 4 tests for GalaxyService. Do you mean we need more?
2. I agree, I'm going to remove these things later today.

Andrey A.

[lassewesth]

@Andrey:
Regarding 1), I think GalaxyService should go, and with it the tests of course. Instead, I would use the old WorldRepositoryTests as the driver (i.e. no main method to run this at all), and have a couple of illustrative tests in there. Those tests could look very similar to the GalaxyServiceTests, but without the superfluous GalaxyService of course.
Lasse

[agibalov]

You've never seen it before. Brand new. Exclusive. Only 4 files of code. It's time for the future!

• Removed DTO
• Removed mapping
• Kept service
• Reverted tests

I'm firmly against of removing the service and moreover, now there is a real argument:

findAllByNumberOfMoons(1)

is much more readable than

findAllByPropertyValue("moons", 1)

Hope that's fine now :-)

[jexp]

Thanks Andrey!