Help with Coding Style

[Christoph Friedrich]

Hello there,

I am new to Perl and I am not sure if my Coding Style is good.
Could some of you please check if my Coding Style is good or tell me
what I should change to get a good Coding Style?
You can find 2 files of my current project under
http://www.christophfriedrich.de/code/
Please check it so that I can write good code on bigger projects.

Thank you and greetings
Christop

[John W. Krahn]

Christoph Friedrich wrote:
> Hello there,

Hello,

> I am new to Perl and I am not sure if my Coding Style is good.
> Could some of you please check if my Coding Style is good or tell me
> what I should change to get a good Coding Style?
> You can find 2 files of my current project under
> http://www.christophfriedrich.de/code/
> Please check it so that I can write good code on bigger projects.

> # Initialise allowed array to all numbers allowed
> for (my $i = 1; $i <= $size; $i++)

That is usually written as:

for my $i ( 1 .. $size )

> {
> $self->{'allowed'}[$i] = 1;
> }

In Perl arrays start at 0 so you are skipping $self->{'allowed'}[0] in
your initialization.

You can do that without a loop like:

$self->{ allowed } = [ ( 1 ) x $size ];

Or:

@{ $self->{ allowed } } = ( 1 ) x $size;

> # Initialise cells
> for (my $row = 1; $row <= $size; $row++)

That is usually written as:

for my $row ( 1 .. $size )

> {
> $cells[$row] = ();

What do you think that you are assigning to that scalar value? That
makes no sense.

> for (my $col = 1; $col <= $size; $col++)

Same as above:

for my $col ( 1 .. $size )

> {
> $cells[$row][$col] = DragonNet::Games::Sudoku::Board::Cell->new(size => $size);

In Perl arrays start at 0 so you are creating an array of ( $size + 1 )
* ( $size + 1 ) with nothing in row 0 or column 0.

> }
> }

John
--
The programmer is fighting against the two most
destructive forces in the universe: entropy and
human stupidity. -- Damian Conway

[Christoph Friedrich]

Thank you for your fast answer.

John W. Krahn schrieb:

> Christoph Friedrich wrote:
>> Hello there,
>
> Hello,
>
>> I am new to Perl and I am not sure if my Coding Style is good.
>> Could some of you please check if my Coding Style is good or tell me
>> what I should change to get a good Coding Style?
>> You can find 2 files of my current project under
>> http://www.christophfriedrich.de/code/
>> Please check it so that I can write good code on bigger projects.
>
>
>> # Initialise allowed array to all numbers allowed
>> for (my $i = 1; $i <= $size; $i++)
>
> That is usually written as:
>
> for my $i ( 1 .. $size )

OK ^^ I think that is my PHP Style that I couldn't unlearn that fast ^^

>
>> {
>> $self->{'allowed'}[$i] = 1;
>> }
>
> In Perl arrays start at 0 so you are skipping $self->{'allowed'}[0] in
> your initialization.
>
> You can do that without a loop like:
>
> $self->{ allowed } = [ ( 1 ) x $size ];
>
> Or:
>
> @{ $self->{ allowed } } = ( 1 ) x $size;

That is good to know

>
>
>
>
>
>> # Initialise cells
>> for (my $row = 1; $row <= $size; $row++)
>
> That is usually written as:
>
> for my $row ( 1 .. $size )
>
>> {
>> $cells[$row] = ();
>
> What do you think that you are assigning to that scalar value? That
> makes no sense.

I think that I assign an empty Array to $cells[$row]. But I'm not sure
if that is needed.

>
>> for (my $col = 1; $col <= $size; $col++)
>
> Same as above:
>
> for my $col ( 1 .. $size )
>
>> {
>> $cells[$row][$col] =
>> DragonNet::Games::Sudoku::Board::Cell->new(size => $size);
>
> In Perl arrays start at 0 so you are creating an array of ( $size + 1
> ) * ( $size + 1 ) with nothing in row 0 or column 0.

OK I rethink this parts of code so that 0|0 is not undef.
>
>> }
>> }
>
>
>
>
> John
My biggest problem are needed parameters in functions.
Like this:

my ($self, %options) = @_;
my $row = $options{'row'} or die "Parameter row not specified!";
my $col = $options{'column'} or die "Parameter column not specified!";
my $value = $options{'value'} or die "Parameter value not specified!";

I'm not sure if it is OK to use a die if one of these parameters are not
given.
Maybe it is better to return a false value? I'm not sure.

Greets
Christoph

[Peter Scott]

On Fri, 25 Dec 2009 16:57:04 +0100, Christoph Friedrich wrote:
> I am new to Perl and I am not sure if my Coding Style is good. Could
> some of you please check if my Coding Style is good or tell me what I
> should change to get a good Coding Style? You can find 2 files of my
> current project under http://www.christophfriedrich.de/code/ Please
> check it so that I can write good code on bigger projects.

Nice choices of names.

my $self = bless({}, ref ($class) || $class);

This is cargo cult programming for object construction. The actual
utility of a constructor that can work both as a class method and some
sort of cloning function is severely limited and copying this style leads
to fuzzy thinking about object class design.

$self->{'size'} = $size;

Hash keys that are 'well-behaved' are better off unquoted, the code
becomes easier to read.

$cells[$row][$col] =
DragonNet::Games::Sudoku::Board::Cell->new(size => $size);

You have very long package names. Consider using http://search.cpan.org/
~ovid/aliased-0.30/lib/aliased.pm to make the code more succinct.

$cells[$row] = ();

An unusual idiom for setting a scalar to undef. However, since that value
ends up being a reference to an array, this line is unnecessary.
Autovivification will take care of you.

--
Peter Scott
http://www.perlmedic.com/
http://www.perldebugged.com/
http://www.informit.com/store/product.aspx?isbn=0137001274