High Quality Apex: Code Review #2

global class CustomIterable 
// "Global" is often used in Salesforce example code, when usually,
// public is a better option. A good practice is to make code only as
// accessible as it needs to be. No more
   implements Iterator{ 
// A class comment would help the reader understand the purpose and
// responsibilities of this class

   List accs {get; set;} 
// "accounts" would be a clearer property name
// This syntax really doesn't buy us anything. "List accounts;" would
// be a bit simpler to follow
   Integer i {get; set;} 

   public CustomIterable(){ 
// Should the "next" and "hasNext" methods really be globally
// accessible, but not the constructor? Probably not. My preference
// would be to make the class and methods all "public"
       accs = 
       [SELECT Id, Name, 
       NumberOfEmployees 
       FROM Account 
       WHERE Name = 'false']; 
       i = 0;
// Consider making this more selective, adding a limit, or at least
// commenting on why there will never be too many matching records 

// Consider putting the SOQL query all on one line. This is easy to
// read and requires less vertical scrolling

// There is never a need to query the Id of a record. It's included
// implicitly
   }   

   global boolean hasNext(){ 
       if(i >= accs.size()) {
// Use consistent whitespace
           return false; 
       } else {
           return true; 
       }
// Consider simplifying to "return (i < accs.size());"
   }    

   global Account next(){ 
       // 8 is an arbitrary 
       // constant in this example
       // that represents the 
       // maximum size of the list.
       if(i == 8){return null;}
// Why use an arbitrary constant and the 4-line comment above? The
// correct code is "if (!hasNext())"

// Put the constant first in comparisons to avoid a bug due to a type
// (e.g. "8 == i")

// Put each statement on a new line to improve readability, debugging,
// etc.
       i++; 
       return accs[i-1]; 
   }
// Better: public Account next() { return hasNext() ? accs[i++] : null; }
}

// A class comment would add to maintainability
global class batchClass implements Database.batchable{
// "Global" is often used in Salesforce example code, when usually,
// public is a better option. A good practice is to make code only as
// accessible as it needs to be. No more

// Use consistent capitalization. Salesforce's usual pattern is to
// capitalize classes and interfaces (i.e. "BatchClass, Batchable")

// Consider using a more descriptive class name (i.e.
// "AccountUpdateBatch")
   global Iterable start(Database.batchableContext info){
// A more meaningful parameter name might be "context"
       return new example(); 
   }
// Whitespace - a blank line between methods is common practice and adds to readability
   global void execute(Database.batchableContext info, List scope){
// Capitalize names of all classes (i.e. "BatchableContext")
       List accsToUpdate = new List();
// "accountsToUpdate" is easier to read and remember
       for(Account a : scope){ 
           a.Name = 'true'; 
           a.NumberOfEmployees = 69;
// Consider moving constants to custom labels, custom settings, custom metadata, or top-of-file constants 
// The code isn't clear on this point, but make sure we're not updating records that aren't changing (i.e. Name is already 'true' and NumberOfEmployees is already 69)
           accsToUpdate.add(a); 
       } 
       update accsToUpdate;
// Error handling should be discussed here. Is this the handling desired by the client?
   }     
   global void finish(Database.batchableContext info){     
   } 
}