Please add the missing fields to the constructors PayloadGenerator...

[AlexandreZaytsev]

Hi,
please add the "body" field to the constructors where it is used
by the
public class Mail: Payload
public class SMS: Payload
public class MMS : Payload

))
let's bring everything to one style!
if there is a tag in the qr string, then it should be described in the constructor

Thanks

[codebude]

Hi Alexandre,

Im sorry but I'm not sure if I got you right...
What do you mean by bring body to the constructor? The message/content variables are already part of the constructors of these three classes. Or do you mean that I should harmonize them by renaming them all body instead of message/subject/subject?

The part of your request about the "tag" is unclear to me. Can you explain it in different words or give some examples?

[AlexandreZaytsev]

I meant the following ...
look from the side of automatic code generation (reflection) based on the existing data structure (black box) there is
no check for valid parameters

for example, the constructor
incorrect (there is no check for valid values) used only MailEncoding.SMTP

  public Mail(string mailReceiver, MailEncoding encoding = MailEncoding.MAILTO)
via the method 
  public override string toString()
gives us the following strings for
  switch (this.encoding)
    case MailEncoding.MAILTO: 
      mailto:?subject=&body =                  - not correct because  **subject body**  is not used here     
    case MailEncoding.MATMSG:
      MATMSG:TO:;SUB:;BODY:;;                  - not correct because **SUB BODY** is not used here
    case MailEncoding.SMTP:
       SMTP:::                                                - correct	

incorrect (there is no check for valid values) used MailEncoding.SMTP and (MailEncoding.MAILTO and MailEncoding.MATMSG witout body tag)

   public Mail(string mailReceiver, string subject, MailEncoding encoding = MailEncoding.MAILTO)
via the method 
  public override string toString()
gives us the following strings for
  switch (this.encoding)
    case MailEncoding.MAILTO: 
      mailto:?subject=&body =                    -  not correct because **body**  is not used here 
    case MailEncoding.MATMSG :   
      MATMSG:TO:;SUB:;BODY:;;                    - not correct because **BODY** is not used here
    case MailEncoding.SMTP:
      SMTP:::                                                  - correct

correct

   public Mail(string mailReceiver, string subject, string message, MailEncoding encoding = MailEncoding.MAILTO)
via the method 
  public override string toString()
gives us the following strings for
  switch (this.encoding)
    case MailEncoding.MAILTO: 
      mailto:?subject=&body =                     - correct
    case MailEncoding.MATMSG:   
      MATMSG:TO:;SUB:;BODY:;;                    - correct
    case MailEncoding.SMTP:
      SMTP:::                                                   - correct

maybe this is all a very specific approach - but nevertheless

ps
additionally, for the same purposes - it would be nice to use attributes (Metadata) containing descriptions of parameters and methods in the class code - again for the purpose of automatic code generation from code (to display tooltips controls), for
example

// Class with the Author attribute.  
[Author("Raffael Herrmann")]
...
public class User
{
[Required(ErrorMessage ="User ID not set")]
public string Id { get; set; }
[Required(ErrorMessage ="User name not specified")]
[StringLength(50, MinimumLength=3, ErrorMessage = "Invalid name length")]
public string Name { get; set; }
[Required]
[Range(1, 100, ErrorMessage = "Invalid age")]
public int Age { get; set; }
etc.

[codebude]

Ok, I think I got what you mean. So you mean for the constructors where message won't be set, the ouput string shouldn't contain an empty body (mailto:[email protected]?subject=Test&body=) but no body at all (mailto:[email protected]?subject=Test), correct?

I wouldn't describe the actual behaviour as wrong, because passing an empty string as mail body is practically (for the end user) the same, then passing no body at all and in addition it's syntactically valid. But from a technical point of view, you're right. Passing an empty body makes the payload unnecessarly longer than it must be. So I'll improve/rewrite those payload generators next week, to supress unset body/subject fields.

Regarding the second request - the attributes: I'll have a look into it, but aren't those meant to be used with fields/properties of (data model) classes? If we take the Mail-class for example (see here) - it has no public fields/properties, but all information is passed via the constructor. So, if validation work is done, it should take place in/at the constructor. Can we attach those attributes (Required, Range, ...) also to function parameters of the constructor?

[AlexandreZaytsev]

1. (mailto:[email protected]?subject=Test), correct - yes
2. unfortunately, I will not tell you about the attributes in detail (little experience). but through Reflection, you can read all the attributes from the metadata (including the covered ones)

in my application,
connecting to metadata QRCoder.dll PayloadGenerator here
selected the constructors object in the metadata here
if the constructors were without parameters, I read the properties, if with parameters, the fields

      //constrictor without parameters = there is 'no constructor'
            IEnumerable queryProp = from t in ctor.ReflectedType.GetProperties(BindingFlags.Instance | BindingFlags.Public | BindingFlags.DeclaredOnly)
                                    where !t.IsDefined(typeof(ObsoleteAttribute), true)
                                    select GetGUITreeNode(t.Name, t.PropertyType, null, nestingLevel, parentName);

            //constrictor with parameters = there is 'constructor'
            IEnumerable queryParam = from t in ctor.GetParameters()
                                     where !t.IsDefined(typeof(ObsoleteAttribute), true)
                                     select GetGUITreeNode(t.Name, t.ParameterType, t.DefaultValue, nestingLevel, parentName);
            //Deferred Execution
            foreach (object Param in ctor.GetParameters().Length == 0 ? queryProp : queryParam) { }  //run function from query

then I analyzed the incoming data here
GetGUITreeNode(t.Name, t.ParameterType, t.DefaultValue, nestingLevel, parentName);
where and can I read the attributes of closed and open type properties
and find ToString ​here
public string GetInvokeMember(object initСtor, ConstructorInfo ctor, List<InvokeError> errorList)
and run here
public object GetInvokeCtor(ConstructorInfo ctor, Dictionary<string, object> cntrlFromForm, List<InvokeError> errorList)

it will probably be enough to attach attributes only to Properties, Parameters, Methods
this will allow you to check the correctness of default ranges and hints at the stage of data construction for the end application

[codebude]

Hi Alexandre,
as you can see in #349, I slightly refactored the SMS/MMS/Mail payload generators. In case subject or body/message aren't set, the empty paramters like ?body=&subject= won't be added any longer. For NTT Docomo formats I haven't changed anything, because I'm pretty sure, that even if the subject is empty, it should be added to keep the overall syntax/message structure. (In addition it doesn't harm to pass an empty string...)

I did not add any restriction/validation, because it's valid to pass an empty/null value for receiver/subject/message in any combination. (If all three are empty, the mailto-link should just open the default mail editor. If only a subject is set, the mail editor should compose a new mail with the given subject.) All of those combinations are valid and useful use cases. Thus I won't add any validation/restrictions.

Regarding your request to add (custom) attributes: I have thought again about the topic and I won't add those attributes. There is no useful way by using the standard attributes, because nearly all payload generator classes have no public properties, but are set via their constructors. To fullfill your request, I had to add own/custom attributes that then could be parsed by you via reflection. I don't see that there's a general need or in other words: this is a very, very specific and niche use-case. Thus I won't add this feature. Sorry.