0

My Input:

 $brackets = '\left \right \center \right \left \middle';

On the above string I need to replace every \left string into ( opening braces and every \right string into ) closing braces.

I am able to replace in few methods to get the output. However I want to know how can we done this in r modifier. Here is what I have so far

 $brackets=~s{\\(left|right)}{$&=~s/\\left/\(/r && $&=~s/\\right/\)/gr; }ge;

I don't know where could I find my mistake.

Sinan Ünür
  • 116,958
  • 15
  • 196
  • 339
ssr1012
  • 2,573
  • 1
  • 18
  • 30
  • 2
    Perl gurus here believe you should not "cram everything into one regex". However, your question is good all in all. – Wiktor Stribiżew May 18 '17 at 10:55
  • 5
    Putting two more `s///` in the replacement part of another `s///` is plain disgusting. – Sinan Ünür May 18 '17 at 10:57
  • 2
    "Magic regex" is a thing we see all the time - someone tries to 'code up' a regex to do everything. And regex is powerful, so you _usually_ can. It's just... also quite inscrutable, and hard to read and maintain, so usually what you _should_ be doing is breaking down the problem and writing 'real' program code around it. – Sobrique May 18 '17 at 10:59
  • 1
    If you have to deal with just `(` and `)` replacements, you may use [`s/\\((left)|right)/$2 ? '(' : ')'/eg`](http://ideone.com/w1NF1I) – Wiktor Stribiżew May 18 '17 at 11:01
  • @WiktorStribiżew: I believe in this `Experience makes the man perfect`. Everyone can do the normal replacements but I thought to do in some thing different. _Just now I got my expected on your comments._ – ssr1012 May 18 '17 at 11:10
  • 1
    Just so you understand why your regex doesn't work: `/r` modifier returns the new modified (or unchanged) string (this value will always be true here), so your `&&` returns its right side: `s/\\right/\(/`, which means that the left side has absolutely no effect. You could have done `($&=~s/\\right/)/r)=~s/\\left/(/r`. But see that answers: it's a bad idea to do everything with a single complicated regex. – Dada May 18 '17 at 11:22

4 Answers4

6

For this literal question which involves only two substrings to be replaced, you should use two separate s/// as @Patrick85 recommends. The approach below begins to make sense only if you have 3 or more such replacements. In that case, tucking the mapping away in a hash will make your code clearer.

Use a hash to look up replacements:

#!/usr/bin/env perl

use strict;
use warnings;

my %replacement = (
    '\left' => '(',
    '\right' => ')'
);

my $s = '\left \right \center \right \left \middle';
my $t = $s =~ s/( \\ (?: left | right ))/$replacement{$1}/rgx;

print "$s\n$t\n";

There are many problems with trying to cram everything into a single line:

$brackets=~s{\\(left|right)}{$&=~s/\\left/\(/r && $&=~s/\\right/\)/gr; }ge;

It is hard to read. Also, it is impossible to generalize. What happens if you need to map N strings to their replacements? Let's try to write this a little more clearly by adding some whitespace:

$brackets =~ s{
    \\
    (left | right)
}{
    $& =~ s/\\left/\(/r &&
    $& =~ s/\\right/\)/gr; 
}gex;

Even if this did what you wanted, you've now replaced a single s/// coupled with a simple hash lookup with something that does two additional s/// operations for each match in your source string.

For the example string you showed, this one would do eight additional s/// operations.

That is not good.

Sinan Ünür
  • 116,958
  • 15
  • 196
  • 339
  • Nice regex :). Could you please explain your regex. because i do not understand your hole regex, but i want to understand it. For what are the flags 'rx' ? And this part '( \\ (?: left | right ))' confueses me – FrankTheTank_12345 May 18 '17 at 11:08
  • 1
    This regex: my $t = $s =~ s/(\\(left|right))/$replacement{$1}/rg; also works. Now I only need to know what the flag 'x' do and why you use '?:' – FrankTheTank_12345 May 18 '17 at 11:11
  • 3
    `x` tells the regex engine to ignore whitespace (including linefeeds). It makes formatting patterns easier. – Sobrique May 18 '17 at 11:13
  • You can get rid of some redundant code: `use Data::Munge qw(list2re); ... my $pattern = list2re keys %replacements; my $t = $s =~ s/($pattern)/$replacements{$1}/gr;` – melpomene May 20 '17 at 07:12
5

Use two simple substitutions instead of one complicated one:

#!/usr/bin/env perl
use strict;
use warnings;

my $brackets = '\left \right \center \right \left \middle';

$brackets =~ s/\\left/(/g;
$brackets =~ s/\\right/)/g;

print $brackets."\n";

Output:

( ) \center ) ( \middle
Sinan Ünür
  • 116,958
  • 15
  • 196
  • 339
  • Please go through my highlighted line in my question. – ssr1012 May 18 '17 at 10:59
  • 2
    @ssr1012 honestly, why don't you want to do it this way? This is the correct, obvious approach. It's clearer than the regex in your question and the code is shorter, too. Why does the existence of a line break bother you so much? –  May 18 '17 at 11:02
  • `$replacement` you copied the text from the @Sinan Ünür. – ssr1012 May 18 '17 at 11:16
  • For this literal question, just using two `s///` is what I would recommend. Using the external lookup table only begins to make sense once you have three or more replacement. See also [How do I strip blank space from the beginning/end of a string?](https://perldoc.perl.org/perlfaq4.html#How-do-I-strip-blank-space-from-the-beginning%2fend-of-a-string%3f). A pair of simple `s///` is better than a single complicated one. – Sinan Ünür May 18 '17 at 11:56
0

It is fairly easy to just build a list of replacements.

use strict;
use warnings;

my $brackets = '\left \right \center \right \left \middle';

for ( $brackets ) {
    s/\\left/(/g;
    s/\\right/)/g;
}

print $brackets, "\n";

obviously you can still add to the list and remove whitespace as well.

More information about this method:

Gerhard
  • 22,678
  • 7
  • 27
  • 43
  • 1
    It seems pretty clear to me that you do not understand what the for() block does, hence why you are asking "what is going on here" – Gerhard May 18 '17 at 11:40
  • 2
    @dan1111: *"What is going on here, and why did people upvote it?"* That is more indicative of your lack of facility with Perl than of the quality of the author's code. Take a look at [*Switch Statements* in `perldoc perlsyn`](https://perldoc.perl.org/perlsyn.html#Switch-Statements) and you will read ***"The `foreach` is the non-experimental way to set a topicalizer"***. (The experimental way being `given`.) In the spirit of *TMTOWTDI* I think this answer is excellent, and the only detraction is that it is largely a rewrite of an older solution. – Borodin May 20 '17 at 18:24
  • 1
    @Borodin. Thanks a mill for your input. Much appreciated. – Gerhard May 21 '17 at 10:44
  • 1
    @Dads: It's called a *topicaliser*. Read the link in my note to **dan1111**. – Borodin May 21 '17 at 20:35
-4

Just I need to close this my Question. And the answer provided by @Wiktor Stribiżew [in Comment] on my question.

$brackets = '\left \right \center \right \left \middle';

My Request:

$brackets=~s{\\(left|right)}{$&=~s/\\left/\(/r && $&=~s/\\right/\)/gr; }ge;

Answer:

$brackets=~s/\\((left)|right)/$2 ? '(' : ')'/eg;
Community
  • 1
  • 1
ssr1012
  • 2,573
  • 1
  • 18
  • 30
  • Please have a look at [Can I answer my own question?](http://stackoverflow.com/help/self-answer) and come back two days later and check as answered. – help-info.de May 20 '17 at 09:16