Skip to main content

Notifications

Finance | Project Operations, Human Resources, ...
Suggested answer

making code tidier in x++

(2) ShareShare
ReportReport
Posted on by 1,321
Hi,

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
The reason I made the validations in the same method is because if the validation passed, i will have access to the table buffer already and assign the values directly

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;
}
 
Categories:
  • Martin Dráb Profile Picture
    Martin Dráb 230,868 Most Valuable Professional on at
    making code tidier in x++
    There is no single way best for all possible scenarios. If you have a concrete case, share it with us and we can discuss it.
  • .. Profile Picture
    .. 1,321 on at
    making code tidier in x++
    Hi Martin, 

    Then how would you do it? can you please explain further?
     
  • Martin Dráb Profile Picture
    Martin Dráb 230,868 Most Valuable Professional on at
    making code tidier in x++
    Initializing data in a validate* method doesn't sound like a good design to me. The names sais that the method validates the data and doing something else there would be confusing.
     
    By the way, these are instance variables, not global variables.
  • .. Profile Picture
    .. 1,321 on at
    making code tidier in x++
    Hi Martin,

    if let's say i do use the table buffers (custTable and InventSite) inside the createSalesTable() method, would you use global variables like i showed in the question and use them directly inside the create method? or would you do sth else?
  • Suggested answer
    Martin Dráb Profile Picture
    Martin Dráb 230,868 Most Valuable Professional on at
    making code tidier in x++
    You don't need those variables at all. You never read any value from inventSite and you don't have to take AccountNum from custTable either, because you already have the value in _requestContract.AccountNum().

Under review

Thank you for your reply! To ensure a great experience for everyone, your content is awaiting approval by our Community Managers. Please check back later.

Helpful resources

Quick Links

Announcing Our 2025 Season 1 Super Users!

A new season of Super Users has arrived, and we are so grateful for the daily…

Vahid Ghafarpour – Community Spotlight

We are excited to recognize Vahid Ghafarpour as our February 2025 Community…

Congratulations to the January Top 10 leaders!

Check out the January community rock stars...

Leaderboard

#1
André Arnaud de Calavon Profile Picture

André Arnaud de Cal... 292,031 Super User 2025 Season 1

#2
Martin Dráb Profile Picture

Martin Dráb 230,868 Most Valuable Professional

#3
nmaenpaa Profile Picture

nmaenpaa 101,156

Leaderboard

Product updates

Dynamics 365 release plans