l.stelm...@samsung.com (Łukasz Stelmach) writes:

> It was <2013-04-23 wto 17:02>, when Junio C Hamano wrote:
>> Łukasz Stelmach <l.stelm...@samsung.com> writes:
>>
>>> Enable sending patches to NNTP servers (Usenet, Gmane).
>>> ---
>>>
>>> The patch implements support for sending messages to groups on NNTP 
>>> serviers.
>>
>> Cute.
>>
>> A Perl guru might want to encapsulate the differences between $smtp
>> and $nntp codepaths into two Perl modules, but it looks like a good
>> starting point.
>
> You mean *one* perl module like Git::EmailTransport which hides the
> differences.

What I meant was one class to handle SMTP and another for NNTP.

You look at the --protocol option, choose one of these classes, and
initialize an instance of the chosen class.

You can ask the chosen class to instantiate an instance without
if/else cascade like this:

+
+# Transport specific setup
+my ($email_authuser, $email_authpass);
+if ($email_protocol eq 'nntp') {
+    $email_authuser = $nntp_authuser;
+    $email_authuser = $nntp_authuser;
+    @initial_to = @initial_cc = @bcclist = ();
+    $to_cmd = $cc_cmd = undef;
+    $no_cc = $no_bcc = 1;
+} else {
+    $email_authuser = $smtp_authuser;
+    $email_authpass = $smtp_authpass;
+    $newsgroups_cmd = undef;
+}
+

By asking that instance to ask questions to fill in whatever it
needs from the user, you can remove the need of if/elsif cascade
like this part in your patch:

+if ($email_protocol eq 'smtp' && !@initial_to && !defined $to_cmd) {
        my $to = ask("Who should the emails be sent to (if any)? ",
                     default => "",
                     valid_re => qr/\@.*\./, confirm_only => 1);
        push @initial_to, parse_address_line($to) if defined $to; # 
sanitized/validated later
        $prompting++;
+} elsif ($email_protocol eq 'nntp' &&
+        !@initial_newsgroups &&
+        !defined $newsgroups_cmd) {
+       my $newsgroup = ask("Which newsgroups should the message be sent to (if 
any)? ",
+                    default => "",
+                    valid_re => qr/[\x20-\x7f]+/, confirm_only => 1);
+       push @initial_newsgroups, $newsgroup if defined $newsgroup; # 
sanitized/validated later
+       $prompting++;
 }


Naturally, a function like this will become a method of the
instance without if/else cascade:

+sub email_host_string {
+       if ($email_protocol eq 'nntp') {
+               if (defined $nntp_server_port) {
+                       return "$nntp_server:$nntp_server_port";
+               } else {
+                       return $nntp_server;
+               }
+
        } else {
-               return $smtp_server;
+               if (defined $smtp_server_port) {
+                       return "$smtp_server:$smtp_server_port";
+               } else {
+                       return $smtp_server;
+               }
        }
 }

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to