Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free.

One of my colleagues recently interviewed some candidates for a job and one said they had very good Perl experience.

Since my colleague didn't know Perl, he asked me for a critique of some code written (off-site) by that potential hire, so I had a look and told him my concerns (the main one was that it originally had no comments and it's not like we gave them enough time).

However, the code works so I'm loathe to say no-go without some more input. Another concern is that this code basically looks exactly how I'd code it in C. It's been a while since I did Perl (and I didn't do a lot, I'm more a Python bod for quick scripts) but I seem to recall that it was a much more expressive language than what this guy used.

I'm looking for input from real Perl coders, and suggestions for how it could be improved (and why a Perl coder should know that method of improvement).

You can also wax lyrical about whether people who write one language in a totally different language should (or shouldn't be hired). I'm interested in your arguments but this question is primarily for a critique of the code.

The spec was to successfully process a CSV file as follows and output the individual fields:

User ID,Name , Level,Numeric ID
pax, Pax Morgan ,admin,0
gt,"  Turner, George" rubbish,user,1
ms,"Mark \"X-Men\" Spencer","guest user",2
ab,, "user","3"

The output was to be something like this (the potential hire's code actually output this):

User ID,Name , Level,Numeric ID:
   [User ID]
   [Name]
   [Level]
   [Numeric ID]
pax, Pax Morgan ,admin,0:
   [pax]
   [Pax Morgan]
   [admin]
   [0]
gt,"  Turner, George  " rubbish,user,1:
   [gt]
   [  Turner, George  ]
   [user]
   [1]
ms,"Mark \"X-Men\" Spencer","guest user",2:
   [ms]
   [Mark "X-Men" Spencer]
   [guest user]
   [2]
ab,, "user","3":
   [ab]
   []
   [user]
   [3]

Here is the code they submitted:

#!/usr/bin/perl

# Open file.

open (IN, "qq.in") || die "Cannot open qq.in";

# Process every line.

while (<IN>) {
    chomp;
    $line = $_;
    print "$line:\n";

    # Process every field in line.

    while ($line ne "") {
        # Skip spaces and start with empty field.

        if (substr ($line,0,1) eq " ") {
            $line = substr ($line,1);
            next;
        }

        $field = "";
        $minlen = 0;

        # Detect quoted field or otherwise.

        if (substr ($line,0,1) eq "\"") {
            $line = substr ($line,1);
            $pastquote = 0;
            while ($line ne "") {
                # Special handling for quotes (\\ and \").

                if (length ($line) >= 2) {
                    if (substr ($line,0,2) eq "\\\"") {
                        $field = $field . "\"";
                        $line = substr ($line,2);
                        next;
                    }
                    if (substr ($line,0,2) eq "\\\\") {
                        $field = $field . "\\";
                        $line = substr ($line,2);
                        next;
                    }
                }

                # Detect closing quote.

                if (($pastquote == 0) && (substr ($line,0,1) eq "\"")) {
                    $pastquote = 1;
                    $line = substr ($line,1);
                    $minlen = length ($field);
                    next;
                }

                # Only worry about comma if past closing quote.

                if (($pastquote == 1) && (substr ($line,0,1) eq ",")) {
                    $line = substr ($line,1);
                    last;
                }
                $field = $field . substr ($line,0,1);
                $line = substr ($line,1);
            }
        } else {
            while ($line ne "") {
                if (substr ($line,0,1) eq ",") {
                    $line = substr ($line,1);
                    last;
                }
                if ($pastquote == 0) {
                    $field = $field . substr ($line,0,1);
                }
                $line = substr ($line,1);
            }
        }

        # Strip trailing space.

        while ($field ne "") {
            if (length ($field) == $minlen) {
                last;
            }
            if (substr ($field,length ($field)-1,1) eq " ") {
                $field = substr ($field,0, length ($field)-1);
                next;
            }
            last;
        }

        print "   [$field]\n";
    }
}
close (IN);
share

migration rejected from stackoverflow.com Jun 20 at 19:31

This question came from our site for professional and enthusiast programmers. Votes, comments, and answers are locked due to the question being closed here, but it may be eligible for editing and reopening on the site where it originated.

closed as primarily opinion-based by durron597, Snowman, MichaelT, GlenH7, gnat Jun 20 at 19:31

Many good questions generate some degree of opinion based on expert experience, but answers to this question will tend to be almost entirely based on opinions, rather than facts, references, or specific expertise. If this question can be reworded to fit the rules in the help center, please edit the question.

    
Actually I'm confused. I don't understand the subject, the programmer wrote in Perl to parse a CSV file, but how does C factor into this? Do you mean that he is being hired to write C code, and is proficient in Perl? –  Brian Aug 19 '11 at 16:36
1  
@Brian The style of the "Perl" the programmer writes is C-like. Using substr()/while loops instead of a simple little regex, for example. Look at @JohnathanLeffler's example. –  Mateen Ulhaq Aug 20 '11 at 0:32

2 Answers 2

UN-FREAKING BELIEVABLE

You have, literally, a guy that has written several books on programming Perl and is well known in the community, saying that this is acceptable code.. And then 90% of the rest of the posts saying how awful this guy is, and should be a no-hire.

Candidate was given a task, completed the task, submitted clean, working code...... and now people are ripping it because even though it works, they don't like the style and would've done it different.

5'll get you 10 that these guys saying this also work for/in a company that complains that they can't find programmers....

share

Since Perl is C, I don't see an issue.

The most commonly used Perl modules have a C compilation. See DBI. See (YAML|*)::XS, see Template Toolkit. See libmemcached, see YAML::XS So, hell yes, if your developer can write Perl and C, and understand why, you'd be nuts not to hire them.

If you're concerned about the reliability, ask them to write comprehensive [unit] tests.

share
1  
The issue is writing perl using C idioms rather than writing idiomatic perl. Writing if (($pastquote == 0) && (substr ($line,0,1) eq "\"")) { rather than if (not $pastquote and $line =~ m/^"/) { -- I've written C in Lisp, its not a good thing. Write perl. Write C. But don't write C in perl or perl in C. –  MichaelT Jan 23 '14 at 1:13
    
Generally speaking, though, you can do a code review. You can require tests. You can mandate whatever standard you require. My point was that this skill set is a good one, and demonstrates ability. I'm not saying it's best practice or commenting on syntax, simply the question at hand. –  phill Jan 23 '14 at 1:53

protected by MichaelT Jan 23 '14 at 1:10

Thank you for your interest in this question. Because it has attracted low-quality answers, posting an answer now requires 10 reputation on this site.

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.