aws.String("xxx") ?

[mattes]

Why is this lib using aws.String("xxx") instead of just strings "xxx"?

[lsegal]

@mattes aws.String() returns a pointer because nil is a valid input to these parameters. A lengthier discussion can be found in #114 and others.

[jasdel]

Hi @mattes, The aws.String is a helper so literal strings can be used when setting API field pointer values without needing to define a local variable first, because &"xxx" is not valid syntax. If the value is already defined as a local variable &xxxStr can also be used.

The AWS API operations include many optional fields which should not be included in the marshaled request if not set. In order to represent the ternary state for these fields pointers are used. This is done because it is not possible to distinguish between a never set value from a value set to the type's Zero value. The SDK uses pointers for all primitive and struct type fields. In many cases the type's Zero value of a field has meaning, and is different from unset, and a non zero value. All API fields in Input and Output structs are pointers, because even though a field is required today, it may not be required in the future. A breaking change would be required to allow not setting the once required field.

[mattes]

Hmm. I think I understand. Wouldn't it be nicer to use interfaces for things like that though? Like:

func foo(stringOrNil interface{}) {
  ...
}

[lsegal]

@mattes using empty interface types would completely remove all type checking / inference in the language and tooling, and I would imagine this would cause much more confusion in the library and -1's from the community at large where type information is considered extremely useful.

[mattes]

I've been working with the API today and I understand the reason behind using pointers. Honestly, I am not a fan though. It creates very weird constructs of switching back and forth between pointers while integrating with the rest of the code base. Debugging isn't fun neither. I am not saying it's hard. It's just not very aesthetic. Which I think is definitely an argument. :-)

I still think something like map[string]interface{} would work for inputs. Yes, it would remove compiler type checking, but I think it's an reasonable approach in this situation. Return types could still be structs. Not sure how others think about this. But it seems as if I am not the only one who is a little unhappy with the current pointer approach.

[alexaandru]

+1 for the current pointer approach -1 for anything based on interface{}

[omeid]

Why not use methods then?

b := s3.NewObject(bucket, key, map[string]string{"key": key});
err := b.SetACL("ObjectCannedACL");
//Check ACL et al.

[jasdel]

Hi @omeid take a look at #294 and #306. Between the two the cover the topic pretty well.

Since the service APIs are iterating quickly, fields which were previously required may become optional. This recently occurred with EC2 making changing parameter from required to optional. If we had methods for these operations it would be impossible for you not to provide a value for the now optional parameter without making a breaking change to the SDK. Once the SDK is released and ready for production we want to ensure it is stable.

[endophage]

Given the desirability of more idiomatic go (path of least surprise yada yada yada...), are there cases where the empty string, "", would functionally mean something different to nil?

[endophage]

Incidentally, I'm also seeing plenty of places a *string has been used where a []byte would be more appropriate (like the body of SQS Messages). Slices can be nil and that would solve your problems in some cases.

[stevvooe]

@jasdel

This is done because it is not possible to distinguish between a never set value from a value set to the type's Zero value. The SDK uses pointers for all primitive and struct type fields. In many cases the type's Zero value of a field has meaning, and is different from unset, and a non zero value. All API fields in Input and Output structs are pointers, because even though a field is required today, it may not be required in the future. A breaking change would be required to allow not setting the once required field.

The use of *string and others is extremely non-idiomatic, making use of this package more challenging when integrating with existing Go code. Many existing Go packages cope with this just fine and I implore you to study them.

Other than the verbosity required, the main issue with this is the unnecessary performance penalty of the pointer redirect and the extra garbage generated.

Effectively, to use your package, a Go developer must depart from the style commonly used throughout the standard library and every other package.

[jasdel]

Thanks for the feedback. We definitely want to provide the best idiomatic experience within the context of the AWS service APIs. How the SDK exposes its fields is something we really want to make sure is a good experience prior to releasing the SDK.

[jasdel]

@endophage The sqs.SendMessageInput.MessageBody actually must be a valid unicode strings, any non-unicode binary data should be unicode escaped. If the API used a []byte instead of string it may imply that binary data is valid for the message body, which isn't the case. (#339) With that said, if you're looking to include binary or numbers within a message not escaped sqs.SendMessageInput.MessageAttributes using sqs.MessageAttributeValue is the best option to specify the additional binary or number data.

[jasdel]

@stevvooe Since many fields within the the API operations are optional a concept is needed to identify if the field was set to a non-zero value, not set at all, or set to the zero value for the type. When using by value there is no way to distinguish the difference between set to zero value, and not set at all. Since zero value for primitive types are valid API operation values, the common omitempty for zero value pattern is not valid. In order to do so, extra information is needed.

To provide that extra information there are four common ways this could be done.

• Use pointers instead of values. This was the original proposed solution Go applications began to use to provide dynamic not-set marshalling for concrete structure fields. Non-string primitives such as bool and int64 highlight this issue more than string within the context of the SDK since using zero value for omitempty would prevent setting a field to 0 or false. But like you mentioned the usage of pointers will increase number of allocations, even more so for operations with many fields.
• Use map[string]interface{}. This would remove the need for pointers, but also remove the highly valued compile time type checking. In addition, it would remove any type ahead assistance various IDEs may provide. In addition field names would need to be exposed as consts in order to prevent typo's in field names.
• Use function params for required fields. e.g. svc.GetObject("bucketName", "keyName") If the service APIs were static this would be a great pattern for the SDK to follow. But since the service API operations are always evolving fields which were once required can become optional. Required fields becoming optional or optionally required is not uncommon. This pattern would require the SDK to make breaking changes to allow users to not to set previously required fields. This is a perfect pattern to be written as a level above the SDK, since users would be able to control when they upgrade, and choose to update their helper functions as they need. While also being able to update the SDK for new features without breaking their existing code.
• Wrap primitives struct. As an alternative to pointers we looked into using structs to wrap primitives. The struct would embed the needed tri-state information. With this pattern we could remove the need for pointers of primitives, but it would still require a aws.String("xxx") like method to be able to set the value for a field inline. At the same time it would remove the ability to set a field with the variable directly, such as Bucket: &bucketName which is an option today. These wrappers also have the added benefit of removing the possibility of dereferencing a nil pointer for unset response output structs.StringType below is an abbreviated example of this tri-state wrapper.

type StringType struct {
    s string
    set bool
}
func String(s string) StringType { return StringType{s: s, set: true} }

Of these options primitive pointers and wrapping structs are the two which provide the most flexibility and stability for their cost and ease of use. Any feedback on these methods you have are more than welcome. I have experimented a few times (#276, #294) with using the primitive wrappers, but have not pushed it forward because it removes the ability to use &fieldValue and requires a helper func aws.String("xxx"). The helper func is required because initializing the struct by itself such as aws.StringType{S: s, Set: true} would require setting the Set field to be provided also, and would be a big risk for users thinking they set a value when they really didn't.

[stevvooe]

Since many fields within the the API operations are optional a concept is needed to identify if the field was set to a non-zero value, not set at all, or set to the zero value for the type. When using by value there is no way to distinguish the different between set to zero value, and not set at all.

I disagree with this premise. Zero-valued types are commonly used without adding such complexity across the Go community. I'm confused as to why this doesn't work for this project. Please examine this premise and work from there.

Could you give some concrete examples in the API where an empty string, zero-value number or other zero-valued type needs to be distinguished? Looking at Config, as an example, not one field cannot be correctly detected via zero-valued types. A cursory look over other types shows this to be unnecessary, as well.

Take a peak at this quote from Best practices for a new Go developer:

You’ll work with the strengths of the languages more easily if you don’t try to do C-like things (allocation on the heap, pointers to everything) and treat the stack and pass-by-value as cheap operations for most purposes. I’ve seen big performance improvements just by simplifying heap usage and replacing it with stack usage. Go is designed to do the right thing for programs that look very straightforward, so if you feel like you’re doing something clever, it’s potentially working against both you and the runtime.

While this quote isn't perfectly apropos, please examine whether or not approaching the problem this way is truly necessary. Especially when multiple experienced Go developers have suggested this might not be the right approach.