simple-scan-team team mailing list archive

[Robert Ancell <robert.ancell@xxxxxxxxxxxxx>]

Review: Needs Fixing

Just a quick first review as I'm busy with the Ubuntu 14.04 release...

- Make use of automatic variable typing, i.e.
List<Page> copy ;
copy = new List<Page> ();
Can be done as:
var copy = new List<Page> ();
and
var ix = 0 ;
for ( ix = 0 ; ix != cnt ; ix ++ ){
can be done as
for (var i = 0; i != book.n_pages; i++)

- You have some tab characters in the code, please replace them with four spaces.

- Check for existing coding style, i.e.
repage_menuitem.set_sensitive ( enable_repage );
should be:
repage_menuitem.set_sensitive ( enable_repage );

- Could you rework the following statement, it is a bit confusing:
57	+	    uint cnt = book.n_pages ;
58	+	    bool enable_repage = ( 0 == ( cnt & 0x1 )) && 
59	+				( cnt > 2 )
60	+					  ;
i.e. something more like
/* Only show repaging option when we have an even number of pages and enough pages to do it */
var enable_repage = book.n_pages > 2 && book.n_pages % 2 == 0;

- 
-- 
https://code.launchpad.net/~wmack-y/simple-scan/page-reorder/+merge/214473
Your team Simple Scan Development Team is subscribed to branch lp:simple-scan.