My question here is about making the code more tidy.
As you can see in createSalesTable() method, i assign lots of values. And there are two fields that I make validations for before filling them in SalesTable, which are custAccount and InventSiteId
class SalesOrderService
{
public ResponseContract createSalesOrder(RequestContract _requestContract)
{
SalesTable salesTable;
System.Exception ex;
ResponseContract responseContract = new ResponseContract ();
if(_reqContract)
{
try
{
changecompany(_reqContract.DataArea())
{
ttsbegin;
salesTable = this.createSalesTable(_requestContract);
//logic
ttscommit;
}
}
catch(ex)
{
//logic
}
}
else
{
//logic
}
return responseContract;
}
private SalesTable createSalesTable(RequestContract _requestContract)
{
SalesTable salesTable;
CustTable custTable;
select firstonly AccountNum, Party from custTable where custTable.AccountNum == _requestContract)();
if(!custTable)
{
throw Error("@InvalidCustomerAccount");
}
else
{
salesTable.CustAccount = custTable.AccountNum;
}
salesTable.SalesId = NumberSeq::newGetNum(SalesParameters::numRefSalesId()).num();
salesTable.initValue();
salesTable.initFromCustTable();
if(_reqContract.Currency())
{
salesTable.CurrencyCode = _requestContract.Currency();
}
salesTable.ReceiptDateConfirmed = _requestContract.ReceiptDateConfirmed();
salesTable.ReceiptDateRequested = _requestContract.ReceiptDateRequested();
salesTable.CustomerRef = _requestContract.CustomerReference();
salesTable.PurchOrderFormNum = _requestContract.PurchOrderFormNum();
InventSite inventSite;
select firstonly inventSite where inventSite.SiteId == _requestContract.SiteId();
if(inventSite)
{
salesTable.InventSiteId = _requestContract.SiteId();
salesTable.modifiedInventSiteFromParent();
}
else
{
throw Error (strFmt("@SiteIdNotFound", _requestContract.SiteId()));
}
if(salesTable.validateWrite())
{
salesTable.insert();
}
else
{
throw Exception::Error;
}
return salesTable;
}
Now, i think it's better to make validations in a sepearate method, but my question how would you do it, to not have to do another select when setting the values, would you put CustTable and InventSite as global varaibles like this? or would you do it differently?
class SalesOrderService
{
private CustTable custTable; //global
private InventSite inventSite; //global
public ResponseContract createSalesOrder(RequestContract _requestContract)
{
SalesTable salesTable;
System.Exception ex;
ResponseContract responseContract = new ResponseContract ();
if(_reqContract)
{
try
{
changecompany(_reqContract.DataArea())
{
ttsbegin;
this.validateSalesTable(_requestContract) //new method
salesTable = this.createSalesTable(_requestContract);
//logic
ttscommit;
}
}
catch(ex)
{
//logic
}
}
else
{
//logic
}
return responseContract;
}
private void validateSalesTable(RequestContract _requestContract)
{
select firstonly AccountNum, Party from custTable where custTable.AccountNum == _requestContract.AccountNum();
if(!custTable)
{
throw Error("@InvalidCustomerAccount");
}
select firstonly inventSite where inventSite.SiteId == _requestContract.SiteId();
if(!inventSite)
{
throw Error (strFmt("@SiteIdNotFound", _requestContract.SiteId()));
}
}
private SalesTable createSalesTable(RequestContract _requestContract)
{
SalesTable salesTable;
salesTable.CustAccount = custTable.AccountNum;
//logic to fill more fields
salesTable.InventSiteId = _requestContract.SiteId();
salesTable.modifiedInventSiteFromParent();
if(salesTable.validateWrite())
{
salesTable.insert();
}
else
{
throw Exception::Error;
}
return salesTable;
}