Written by

Question Manoj K · Mar 3, 2017

Please review the code and give me some suggestion to improve coding standard.

I created a simple CRUD operation using Rest Services in Cache.

Class EX.example Extends %CSP.REST{XData UrlMap{<Routes><Route Url="/insert/:loc/:ssn/:name" Method="GET" Call="hello"/><Route Url="/update/:key/:loc/:ssn/:name" Method="GET" Call="iam"/><Route Url="/delete/:key" Method="GET" Call="there"/><Route Url="/findid/:key" Method="GET" Call="now"/></Routes>}ClassMethod hello(name As %String, ssn As %String, loc As %String) As %Status{s mName=$zcvt(name,"U"),mKey=ssn,mLoc=locs ns=$namespaceznspace "USER"try{s obj=##class(Sample.Classes).%New()s obj.name=mName, obj.SSN=mKey, obj.Location=mLocw "----Great----"s Status=obj.%Save()}catch{w "Sorry Unable to insert"}znspace nsquit $$$OK}ClassMethod iam(id As %Integer, name As %String, ssn As %String, loc As %String){s mId=id,mName=$zcvt(name,"U")s ns=$namespaceznspace "USER"try{if (##class(Sample.Classes).%ExistsId(mId)){s objj=##class(Sample.Classes).%OpenId(mId)w objj.%Id(),objj.Locations objj.Location=loc,objj.name=mName,objj.SSN=ssns Status=objj.%Save()w "----Wow Great---------"}else{w "There is no ",id," is present"}}catch{w !,"Sorry Unable to update the Id:",mId}znspace nsq $$$OK}ClassMethod there(id As %Integer){S mId=ids ns=$namespaceznspace "USER"try{if (##class(Sample.Classes).%ExistsId(mId)){s obj=##class(Sample.Classes).%DeleteId(id)w "Deleted Successfully"}else{w "There is no ",id," is present"}}catch{w "Sorry Unable to delete the Id:",mId}znspace nsq $$$OK}ClassMethod now(id As %Integer){S mId=ids ns=$namespaceznspace "USER"try{if (##class(Sample.Classes).%ExistsId(mId)){s objj=##class(Sample.Classes).%OpenId(mId)w "ID:",objj.%Id()," Name:",objj.name," SSN:",objj.SSN," Location:",objj.Location}else{w "There is no ",id," is present"}}catch{w "Sorry Unable to find the Id:",mId}znspace nsq $$$OK}}

Created a simple class with three properties and performed the CRUD operation for below Class Definition

Class Sample.Classes Extends %Persistent{Property name As %String;Property SSN As %String;Property Location As %String;

}

Please review the code and give me some suggestion to improve coding standard.

Thanks

Comments

John Murray · Mar 3, 2017

Here are some initial suggestions / comments (no particular priority):

  1. Standardize on capitalization in naming. For example classnames EX.example and Sample.Classes, and property names name and Location.
  2. Use singular for your classname, e.g. Class rather than Classes
  3. Use meaningful classnames, e.g. Sample.Employee rather than Sample.Classes
  4. Use meaningful method names. e.g. insert or Insert rather than hello
  5. Avoid namespace switching unless really necessary. Presumably in your case the Sample.Classes class is in the USER namespace and the EX.example is somewhere else (maybe you wrote it in your SAMPLES namespace).
  6. For a REST interface, consider returning results in a structured form, e.g. JSON.
  7. Make good use of indenting. Perhaps you already have done this and the post to DC has mangled it.
  8. Avoid using Z-commands, $Z-functions and $Z if non-Z equivalents are available. For example, SET $NAMESPACE="USER" instead of ZN "USER". Note that you can also preserve the current $NAMESPACE value using NEW $NAMESPACE which will automatically reinstate the value when the stack level is exited.

I hope this is a useful start.

0
Mark Hanson  Mar 3, 2017 to John Murray

The biggest issue I saw is that when you call %Save() you are returning the Status code into variable 'Status' which is good, but then this variable is totally ignored. So if you save an object which does not for example pass datatype validation the %Save will return an error in the Status variable but the caller will never know the save failed or why the save failed.

In addition %DeleteId does not return an oref, it returns a %Status code, so you need to check this to see if it is reporting an error and report this to the caller if it does also.

0
Manoj K · Mar 3, 2017

It accepts only Get method. If I give another method, it shows 'Method not allowed'.

Why the method doesn't accept the post or put or delete method?

0
Dmitry Maslennikov  Mar 3, 2017 to Manoj K

Because, your UrlMap define only GET method, If you need more, you should add it to the same or any other methods.

And with CRUD, you should not put delete/insert inside URL, just use different HTTP methods with the same URL.

0
Eduard Lebedyuk · Mar 3, 2017

To add, names of REST arguments in URL should correspond and be in the same order, as method arguments.

Here's a Caché ObjectScript style guidelines.

0