Hi predrag,

Welcome to Perl and PerlMonks :-)

Just a note on terminology: Those things in braces are typically called "blocks" (Update 2: not everything in braces is a block, though, for example there are anonymous hashes). "Loops" are normally for/foreach, while, and until, each of which have their own "blocks". So what you've got there are three if/elsif/else structures, with a total of four blocks. (Update: See Compound Statements)

There's nothing obviously wrong with the code you showed. Whether or not it's "correct" depends on whether your program works or not :-)

The best way to know whether your program works or not is tests. That is, a set of test input data and an expected output to which you can compare the program's actual output (there are ways to automate this, but that's a different topic). In your tests, it's useful to test as many branches of your code as possible, by setting up your test input data so that as many of the if/elsif/else blocks as possible are executed.

Perhaps we could give you some more comments if you could show a slightly longer, more representative sample - for example, something that includes the outer foreach loop and some short example data that it operates on (see also SSCCE). This is just a wild guess since I don't know anything at all about the structure of your input data and what you're doing with it, but perhaps one of these two variants of the logic might be more appropriate:

if ($char eq "a") {
    ...
}
elsif ($char eq "b") {
    ...
}
if ($k==2)  {
    ...
}
else {
    ...
}

# OR

if ($char eq "a") {
    ...
    if ($k==2)  {
        ...
    }
    else {
        ...
    }
}
elsif ($char eq "b") {
    ...
}
[download]

Regards,
-- Hauke D

Thanks a lot to everyone. I've already seen perlmonks as a friendly place but am really more then pleasantly surprised with so fast answers and warm welcome. Except Perl and a little bash scripting I wasn't in programming, only a long ago I've learned some Fortran but forgot a lot. So it was a prety strange for me to see that starting with Perl is not so difficult for a beginner as some people write on the web, but of course, I also see how Perl is complex and powerful. Anyway, I became addicted to do something in Perl everyday...

I will remember terms and other suggestion, in any case will try to combine the first and last block as you suggested.

I understand that nobody can help more seriously if there is not more of the code, but for the beginning, the most important for me was whether it is ok or not.

The code I've sent is simplified because I wanted to concentrate just on one question. It is a part of the script of around 160 lines, that converts a html page from Serbian Latin to Cyrillic alphabet. That script works well, gives desired result (only for simple html pages). The task of these if blocs in foreach loop is to divide html code from the page content that will be converted. I know about Parser module and already installed and tried some simple examples but didn't know how it can help me in this case, so went back to my first solution.

But, similar to my question in this node, although it gives good result, I would like to hear comments on that converter code, even if it is maybe ridiculous solution. I wonder what will be the most appropriate way for that - to start new node or in this one. The whole code could be probably be too much, so I could explain my approach only by words, or give some details of the code. I don't know anyone with whom I could talk about perl, so today is a big change for me.

Hi predrag,

It sounds like the trickiest part of your current solution is probably figuring out whether you're in some part of the HTML code or whether you're in the text, since obviously tags shouldn't be converted to Cyrillic. Unfortunately, parsing HTML is a pretty difficult task (a humorous post about the topic). So I'd like to encourage you to look at one of the parser modules again.

Two classic modules are HTML::Parser and HTML::TreeBuilder, but there are several others, such as Mojo::DOM. If the input is always XHTML, there's XML::Twig and many more XML-based modules. These modules generally break down the HTML into their structure, including elements (<tags>) with their attributes, comments, or text. Some of the modules then represent the HTML as a Document Object Model (DOM), which is also worth reading a little about. It sounds like you only want to operate on text, and maybe on some elements' attributes (such as title="..." attributes).

Operating only on text is relatively easy: for example, in a HTML::Parser solution, you could register a handler on the text event, which does the appropriate conversions, and register a default handler which just outputs everything else unchanged:

use warnings;
use strict;

use HTML::Parser;

my $p = HTML::Parser->new(
    api_version => 3,
    unbroken_text => 1 );
$p->handler(text => sub {
    my ($text) = @_;
    # ### Your filter here ###
    $text=~s/foo/bar/g;
    print $text;
}, 'text');
$p->handler(default => sub {
    print shift;
}, 'text');

my $infile = '/tmp/in.html';
my $outfile = '/tmp/out.html';

open my $out, '>', $outfile
    or die "open $outfile: $!";
# "select" redirects the "print"s
my $previous = select $out;
$p->parse_file($infile);
close $out;
select $previous;
print "$infile -> $outfile\n";
[download]

Operating on attributes will require you to handle opening elements (tags) as well. Note also that the same basic principle I described above applies to the other modules: they all break the HTML down into its components, so that you can operate on only the textual parts, leaving the others unchanged.

BTW, have you seen Lingua::Translit?

Hope this helps,
-- Hauke D

Hi predrag, can we use your first paragraph above in our advertising?

Seriously, Perl is great, isn't it!

You may have had a difficult time experimenting with Parser modules, but it's definitely the right approach. You may be surprised at how simple the code can be. Someone here has probably got experience and good advice for you.

Do not worry about long code example, just use the <readmore></readmore> tags so you don't show it all. See Writeup Formatting Tips.

Also make sure to post some sample input to your program and the output you desire for that input.

edit: added link



The way forward always starts with a minimal test.

Hi predrag, Welcome to the Monastery!

There's nothing wrong with your code per se, but it seems like you could combine the last and the first conditionals into another if ... else. ( On the other hand, as soon as you have an if/else like that, you might want to start using a hash lookup or dispatch table instead. )

( edit: The most important thing is clarity and maintainability ... write code that you can understand "at-a-glance" when you come back to it after a couple of months. As you become more fluent in Perl, your code will become more succinct, but there's nothing wrong with being very explicit. )

Hope this helps!

Just for clarity, the OP should note the seems in

it seems like you could combine the last and the first conditionals into another if ... else.

The middle if...else... clause could change a value used by the last block.

--MidLifeXis