This constructor is written in a confusing way, so you're right to be confused about it. About your questions:

1. The comma operator combines expressions, so shift, shift could just be written as (shift,shift), which is the same as "take the first two elements off of @_". This should have better been written as splice @_, 0, 2.
2. chromatic will wail and rail against ref($class)||$class, but it is a well-entrenched idiom to allow construction of a fresh object from an old object. ref($class) returns the package name of an object or undef if $class is not a reference. This is to make both of these invocations work:

       my $obj = My::Class->new(...);
       my $other_obj = $obj->new(...);
   [download]

   Whether or how that can make sense has been discussed here and elsewhere. In most cases, you can just leave it out.
3. The first parameter of new() is the class name, but it's not hidden, it's there in plain sight:

   My::Class->new("a","b");
   [download]

   is the same as

   My::Class::new("My::Class","a","b");
   [download]

   except that inheritance is not respected in the second case.

4. The while loop is taking the first two elements in @_ and putting them into the hash referenced by $self. Then it removes the first two elements of @_. This loop could have been written more succinct and clear as:

   $self = { @_ };
   # or
   %$self = @_;
   [download]

In short, the whole constructor could have been written as:

sub new {
    my ($class,%args) = @_;
    my $self = \%args;
    bless $self, ref($class)||$class;
};
[download]

You're right except that you need to check the number of parameters to avoid this warning:

Odd number of elements in hash assignment
[download]

I usually use it like this:

    my %args = @_ % 2 ? () : (@_);
[download]

That warning is there for a reason and your code is different from my code.

Your code assumes that the class name has been shifted out of @_ already while my code assumes it's still in @_. I would like to keep the warning just in case somebody (that is, I myself) calls the constructor with the wrong number of arguments. Having the code silently swallow all arguments would be far worse than having a warning telling me what's wrong.

Corion is right that this code isn't sufficient. The goal isn't to avoid warnings; it's the avoid the situations that bring about warnings. Plus, your method will, in most cases, cause the object to be useless. Returning useless things is worse than dying loudly.

In this case, the situation is where the wrong arguments are coming in. Thus, we should probably look at more situations than just the wrong number of arguments. So, we probably want to be looking at the argument names, too. And, maybe even look at validating the values. Maybe we want to allow foo and -foo. In other words, stop hand-rolling and just use Params::Validate.

---

My criteria for good software:

1. Does it work?
2. Can someone else come in, make a change, and be reasonably certain no bugs were introduced?