-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
A (little) more dynamic [DynamoDbProperty] attribute #239
Comments
I have mixed feelings about this feature. It's the same request as we had before #180. The reasons why we didn't implement it originally are still valid. So far I'm not sure that providing a way to make an accidental mistake worth the reward. For example, this workaround [DynamoDbProperty(nameof(Value))]
public int Value { get; set; } doesn't look much worse for me than this code: [DynamoDbProperty]
public int Value { get; set; } That said, I am open to discussion. Maybe I'm missing something and there are more benefits to it rather than saving a few chars. |
I'll have to take a look at #180 (I'm on the phone right now) but I can argue that your suggestion and the suggestion I added both are still supported, it's more a matter of choice. In fact, your suggestion and mine both translate to the same CLR code. The main benefit is much easier copy paste or quick provisioning of multiple properties. If I need to be explicit then I can use the overload to provide the custom name I need tho. Right now I can't see what mistake could the user make by not providing the name explicitly, but I will take a closer look tomorrow at #180, I might be missing something... Thanks for taking a look!! |
Personally I prefer the approach of AWS's SDK when attribute is not required by default. |
At this point we can't remove attributes requirement even if we wanted to because it will break all existing users. We can add the change from this PR to make attributes to infer the name of the property though. I don't like this change personally but it's one of the common requests so I might give up soon :)
to be fair, nothing prevents you from using |
Currently, the
[DynamoDbProperty(name)]
attribute necessitates specifying a property name, which could be more streamlined. An overload that automatically derives the property name from the class property it’s mapped to would greatly simplify the implementation. This would allow developers to maintain clean code while still ensuring that properties are correctly mapped to DynamoDB attributes.The text was updated successfully, but these errors were encountered: