search

sholsapp / gallocy

A distributed shared memory infrastructure.
27 stars 9 forks source link

Started to define an AbstractTransport abstraction. Implemented a UDP… #27

Closed rverdon closed 8 years ago

rverdon commented 8 years ago

Started to define an AbstractTransport abstraction. Implemented the current abstraction for UDP.

sholsapp commented 8 years ago

This looks great...

What is the end client going to look like? Would we ultimately be mixing these things together like UDPClient<UDPTransport>? How might the CurlClient fit in here since that conflates both the client interface and the transport interface? Maybe it doesn't... and is just a weird special case that we inherit because we didn't have this abstraction yet.

sholsapp commented 8 years ago

Also, tests would be great, and let you iterate quickly. :beers:

rverdon commented 8 years ago

Sweet meme

rverdon commented 8 years ago

What is the end client going to look like? Would we ultimately be mixing these things together like UDPClient? How might the CurlClient fit in here since that conflates both the client interface and the transport interface? Maybe it doesn't... and is just a weird special case that we inherit because we didn't have this abstraction yet.

I was imagining a design exactly like you are saying where the UDPClient would use the UDPTransport underneath to actually send and receive UDP packets. The UDPClient would care about who it is sending data to and any error situations (e.g. no data received. Did the packet get lost, if so try and resend it).

The CurlClient might not have a corresponding AbstractTransport underneath and just treat curl as the transport method.

sholsapp commented 8 years ago

You broke the tests. :heart: 

gallocy/include/gallocy/http/transport.h:42:  Missing username in TODO; it should look like "// TODO(my_username): Stuff."  [readability/todo] [2]
gallocy/include/gallocy/http/transport.h:94:  Missing username in TODO; it should look like "// TODO(my_username): Stuff."  [readability/todo] [2]

Style guide checker is cool, right? I want to add a coveralls check, next.

sholsapp commented 8 years ago

What is the end client going to look like? Would we ultimately be mixing these things together like UDPClient? How might the CurlClient fit in here since that conflates both the client interface and the transport interface? Maybe it doesn't... and is just a weird special case that we inherit because we didn't have this abstraction yet.

I was imagining a design exactly like you are saying where the UDPClient would use the UDPTransport underneath to actually send and receive UDP packets. The UDPClient would care about who it is sending data to and any error situations (e.g. no data received. Did the packet get lost, if so try and resend it).

The CurlClient might not have a corresponding AbstractTransport underneath and just treat curl as the transport method.

I like it!

rverdon commented 8 years ago

DOH. I approve of automatic style checking.

Currently, fixing and cleaning things up at the moment.

sholsapp commented 8 years ago

Hey, can you fix the style and squash your commits? I'll merge this then. Look awesome, super happy you're helping out!

rverdon commented 8 years ago

Done.

sholsapp commented 8 years ago

Woo hoo. You're the second committer ever.