3 ms·
Looks like a good start, but from a quick glance it is missing some Go idioms. Some cursory recommendations: - No Makefile please, this project's build process
by jzelinskie 11y ago
Looks like a good start, but from a quick glance it is missing some Go idioms. Some cursory recommendations:
- No Makefile please, this project's build process isn't yet complicated enough to warrant one.
- Rename the App type something more descriptive like Server
- Rename InitApp to NewApp (NewXXX is an idiomatic Go constructor). Likewise with InitWorker
- Make NewWorker a method of an App
- Isolate the AMQPConnection into another library and use the design pattern in database/sql to allow users to create their own implementations of brokers
- Do whatever you can to avoid empty interfaces (aka interface{}). You may have to be clever, but really avoid these at all costs. Using them basically mentions you'll have to sprinkle type assertions everywhere in the code rather than maybe having one place where you unmarshal values.
- richardknop 11y agoThank you very much for advice. Yes, I agree empty interfaces{} are a bad way of approaching this. I am thinking of using reflection in order to achieve this. I will need to think of some clever way to describe the types in the message so I can then use reflection to unpack JSON into proper types and pass them to a task (without making the API too overcomplicated). I will try to do this over next couple of weekends. Hopefully I can come up with some clever and clean API. Isolating AMQPConnection is also a great idea, I am planning to support multiple brokers (Redis, database etc) in the future.
- richardknop 11y agoI did a quick implementation of reflection today so arguments are not empty interfaces now and tasks are just simple functions with properly typed arguments. I will think more about simplifying it further.