function readOnly(count){ }
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
+ Start a Discussion
tantoniotantonio 

Help me improve this controller class with common best practices

I have a functioning controller class with a simple page block visualforce page on top. Although it is functioning I don't believe its very well written yet. I want to mature it and make sure its formed as best it can be without extending the funcitonality. 

 

Can you make some suggestions of what I could do to improve and why it would benefit the class? Thanks!!!

 

Controller Class:

 

Public class listorgs  implements iterator<org__c>{
        public List<org__c> newlist {get; set;}
        public List<org__c> deplist {get; set;}
    	public List<org__c> devlist {get; set;}
        Integer i {get; set;}

        public listorgs(){
            newlist = [select id, name , LoginURL__c, LoginURLs__c , HRM_Installed__c, Description__c , Type__c, 
                       Pod__c, org_id__c, Version_Password__c, Deprecated__c, IELink__c, PW__c, CreatedDate
                       from org__c where Deprecated__c != True and Type__c!='Dev Org'
                       order by type__c asc];
            
            devlist = [select id, name , LoginURL__c , LoginURLs__c, HRM_Installed__c, Description__c , Type__c, 
                       Pod__c, org_id__c, Version_Password__c, Deprecated__c, IELink__c, PW__c, CreatedDate
                       from org__c where Deprecated__c != True and Type__c='Dev Org'
                       order by type__c asc];
            
            deplist = [select id, name , LoginURL__c , LoginURLs__c, HRM_Installed__c, Description__c , Type__c, 
                       Pod__c, org_id__c, Version_Password__c, Deprecated__c, IELink__c
                       from org__c where Deprecated__c = True
                       order by type__c asc];
            i = 0;
                
        }
        Public boolean hasnext(){
            if(i>= newlist.size()){
                return false;
            } else {
                return true;
            }
        }
        Public Org__c next(){
            if(i==100){return null;}
            i++;
            return newlist[i-1];
        }

     //save button
            public pagereference save() { 
            update newlist; 
            update devlist;
            update deplist;
            return null; 
        }
    //New button
    public PageReference newrecord (){
        PageReference pr;
        pr = new pagereference ('https://na14.salesforce.com/a0R/e?retURL=%2Fa0R%2Fo');
        pr.setredirect(true);
        return pr;
    }
}

 

VF Page:

<apex:page controller="listorgs" sidebar="false">

    <apex:form >
    
        <Apex:pageblock >
          <apex:pageblockButtons >
            <apex:commandButton action="{!Save}" value="Save"/>


            <apex:commandButton action="{!newrecord}" value="New"/>
            </apex:pageblockButtons><apex:inlineEditSupport />
        <br/>
        <br/>
            
            <apex:pageblockTable value="{!newlist}" var="obj">
           
               <apex:column >
                   <apex:commandButton onclick="window.open('{!obj.LoginURL__c}')" value="Login" reRender="loginhome">
                       <Apex:outputLink value="{!obj.LoginURL__c}"/>
                   </apex:commandbutton>
               </apex:column>
                
                
               <apex:column headerValue="Username">
                    <apex:commandLink onclick="window.open('{'!obj.name'}')"  reRender="loginhome">
                        <apex:outputLink value="/{!obj.id}">{!obj.name}
                        </apex:outputLink>
                    </apex:commandLink>
               </apex:column>
                                
                <apex:column headerValue="CopyLink" style="width:10px">
                    <apex:outputField style="width:10px" value="{!obj.LoginURLs__c}"/>
                </apex:column>
                        
                <apex:column headerValue="PW" style="width:10px">
                    <apex:outputField style="width:10px" value="{!obj.PW__c}"/>
                </apex:column>
                
                <apex:column headerValue="Type" >
                    <Apex:outputfield value="{!obj.Type__c}"/>
                    
                </apex:column>
                
                <apex:column headerValue="Description">
                    <Apex:outputfield value="{!obj.Description__c}"/>
                
                </apex:column>
                
                <apex:column headerValue="Org ID">
                    <Apex:outputfield value="{!obj.Org_ID__c}"/>
                    
                </apex:column>
                
                <apex:column headerValue="Instance">
                    <Apex:outputfield value="{!obj.POD__c}"/>
                    
                </apex:column>    
                    
                
                <apex:column headerValue="Package Version">
                    <Apex:outputfield value="{!obj.Version_Password__c}"/>
                    
                </apex:column>
                
                <apex:column headerValue="Deprecated">
                    <Apex:outputfield value="{!obj.Deprecated__c}"/>
                    
                </apex:column>
                
                <apex:column headerValue="Created">
                    <Apex:outputfield value="{!obj.CreatedDate}"/>
                    
                </apex:column>
                
            </apex:pageblockTable>
            
            
            
            <apex:pageblockTable value="{!devlist}" var="obj">
           
               <apex:column >
                   <apex:commandButton onclick="window.open('{!obj.LoginURL__c}')" value="Login" reRender="loginhome">
                       <Apex:outputLink value="{!obj.LoginURL__c}"/>
                   </apex:commandbutton>
               </apex:column>
                
                
               <apex:column headerValue="Username">
                    <apex:commandLink onclick="window.open('{'!obj.name'}')"  reRender="loginhome">
                        <apex:outputLink value="/{!obj.id}">{!obj.name}
                        </apex:outputLink>
                    </apex:commandLink>
               </apex:column>
                                
                <apex:column headerValue="CopyLink" style="width:10px">
                    <apex:outputField style="width:10px" value="{!obj.LoginURLs__c}"/>
                </apex:column>
                        
                <apex:column headerValue="PW" style="width:10px">
                    <apex:outputField style="width:10px" value="{!obj.PW__c}"/>
                </apex:column>
                
                <apex:column headerValue="Type" >
                    <Apex:outputfield value="{!obj.Type__c}"/>
                    
                </apex:column>
                
                <apex:column headerValue="Description">
                    <Apex:outputfield value="{!obj.Description__c}"/>
                
                </apex:column>
                
                <apex:column headerValue="Org ID">
                    <Apex:outputfield value="{!obj.Org_ID__c}"/>
                    
                </apex:column>
                
                <apex:column headerValue="Instance">
                    <Apex:outputfield value="{!obj.POD__c}"/>
                    
                </apex:column>    
                    
                
                <apex:column headerValue="Package Version">
                    <Apex:outputfield value="{!obj.Version_Password__c}"/>
                    
                </apex:column>
                
                <apex:column headerValue="Deprecated">
                    <Apex:outputfield value="{!obj.Deprecated__c}"/>
                    
                </apex:column>
                
                <apex:column headerValue="Created">
                    <Apex:outputfield value="{!obj.CreatedDate}"/>
                    
                </apex:column>
                
            </apex:pageblockTable>
            
            
            
            <apex:pageBlockTable value="{!deplist}" var="obj">
            
            <apex:column >
                   <apex:commandButton onclick="window.open('{!obj.LoginURL__c}')" value="Login" reRender="loginhome">
                       <Apex:outputLink value="{!obj.LoginURL__c}"/>
                   </apex:commandbutton>
               </apex:column>
                
               <apex:column headerValue="Username">
                    <apex:commandLink onclick="window.open('{'!obj.name'}')"  reRender="loginhome">
                        <apex:outputLink value="/{!obj.id}">{!obj.name}
                        </apex:outputLink>
                    </apex:commandLink>
               </apex:column>
                        
                
               <!-- <apex:column headerValue="Username" >
                    <apex:outputLink value="{!obj.id}">{!obj.name}</apex:outputLink>
                    
                </apex:column>
                -->
                
                <apex:column headerValue="Type" >
                    <Apex:outputfield value="{!obj.Type__c}"/>
                    
                </apex:column>
                
                <apex:column headerValue="Description">
                    <Apex:outputfield value="{!obj.Description__c}"/>
                
                </apex:column>
                
                <apex:column headerValue="Org ID">
                    <Apex:outputfield value="{!obj.Org_ID__c}"/>
                    
                </apex:column>
                
                <apex:column headerValue="POD">
                    <Apex:outputfield value="{!obj.POD__c}"/>
                    
                </apex:column>    
                    
                
                <apex:column headerValue="Package Version">
                    <Apex:outputfield value="{!obj.Version_Password__c}"/>
                    
                </apex:column>
                
                <apex:column headerValue="Deprecated">
                    <Apex:outputfield value="{!obj.Deprecated__c}"/>
                    
                </apex:column>
                

            
            </apex:pageBlockTable>
        
        
        
        
        
        </Apex:pageblock>
        
    </apex:form>
    
</apex:page>

 

Test Class:

@IsTest
public class TestListorgs{
            static testmethod void testorg(){
        
        //created org__c record
        org__c org1 = new org__c (Name = 'unit test 1',
                                  pw__c = 'password',
                                  Deprecated__c = true);
        insert org1;
        
        //check that the inserted record has correct name
        system.assertequals('unit test 1',org1.name);
        system.assertequals('password',org1.pw__c);
        system.assertequals(True,org1.Deprecated__c);
                

        //Creating i
        integer i=0;
        
        test.starttest();
            //instantiate controller
            listorgs lo = new listorgs();
            lo.hasnext();
                    //Confirm hasnext boolean is true
                    system.assertequals(lo.hasnext(),True);
                    
                   // system.assertequals(lo.next(),lo.hasnext());
            
            //confirm save returns null
       	    system.assertequals(lo.save(),null);
            
            
        	//test next method
            system.assertequals(lo.next(),lo.newlist[i++]);
        test.stoptest();
        
    }           
}