search

calrissian / mango

Common utilities for rapid application development
Apache License 2.0
17 stars 7 forks source link

Method in TypeRegistry to get an alias given a Class #145

Closed cjnolet closed 9 years ago

cjnolet commented 9 years ago

This is needed in Accumulo Recipes

eawagner commented 9 years ago

I may be wrong here, but I think this will actually affect the Java Serialization of the TypeRegistry.

I don't think that it is non starter, but something I want to investigate and that gets documented in the release notes.

cjnolet commented 9 years ago

Hmm. adding a getAlias(Class clazz) would make it not serializable?

eawagner commented 9 years ago

Nevermind it was early, but I finally did a 5 min google search. Simply adding a method won't break serialization compatability, only adding a new field.

eawagner commented 9 years ago

Would it make more sense to have a 'public T decode(Class clazz, U value)' method? What is the usecase for the getAlias method.

cjnolet commented 9 years ago

Check out the accumulo recipes for the use case. I'm not exactly sure why this is an issue for u. I register encoders for classes and I want to know which encoders were registered for a class. On Dec 15, 2014 2:26 AM, "eawagner" notifications@github.com wrote:

Would it make more sense to have a 'public T decode(Class clazz, U value)' method? What is the usecase for the getAlias method.

— Reply to this email directly or view it on GitHub https://github.com/calrissian/mango/issues/145#issuecomment-66957812.

eawagner commented 9 years ago

Its not an issue for me. Sorry, I didn't know why it was needed as it isn't actually documented anywhere (in Mango or AccumucoRecipes). As always this is in the aims of actually having an honest discussion about the proper design of the API.

Maybe in the future, instead of just saying something needs, it we can describe why it needs it so that we don't need to track down these things through a rather large codebase. For future references, after looking through a lot of uses of 'getAlias' I think I found its use in EventGlobalIndexVisitor for the HasLeaf and HasNotLeaf, special usecases.

I think that this method, should have a different signature than getAlias(Class). By overloading the getAlias method, the API forces a special branch condition for the Class Java type. This design means that we remove the ability for someone to register something like a ClassEncoder if they wanted to for some reason. Additionally, changing the name also makes the intent to get the alias for a class explicit instead of for some generic object.

How about 'getAliasForClass' or 'getClassAlias'?

cjnolet commented 9 years ago

That works for me. I woke up and saw this first thing in the morning- I apologizing for my crankiness. I was thining on my way to work that I'd be down for whatever changes you thought appropriate mostly because when I was desigining the Has and hasNot matchers, I was racing through it to verify that it would 1) work and 2) be performance given the structure of the index table.

On Mon, Dec 15, 2014 at 10:34 AM, eawagner notifications@github.com wrote:

Its not an issue for me. Sorry, I didn't know why it was needed as it isn't actually documented anywhere (in Mango or AccumucoRecipes). As always this is in the aims of actually having an honest discussion about the proper design of the API.

Maybe in the future, instead of just saying something needs, it we can describe why it needs it so that we don't need to track down these things through a rather large codebase. For future references, after looking through a lot of uses of 'getAlias' I think I found its use in EventGlobalIndexVisitor for the HasLeaf and HasNotLeaf, special usecases.

I think that this method, should have a different name than getAlias(Class). By overloading the getAlias method, the API forces a special branch condition for the Class Java type. This design means that we remove the ability for someone to do something like a ClassEncoder if they wanted to for some reason. Additionally changing the name it also makes the intent to get the alias for a class explicit instead of for some generic object.

How about 'getAliasForClass' or 'getClassAlias'?

— Reply to this email directly or view it on GitHub https://github.com/calrissian/mango/issues/145#issuecomment-67011360.