New post
Question
· Mar 3, 2017

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

#REST API #Caché

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=loc
s ns=$namespace
znspace "USER"
try{
s obj=##class(Sample.Classes).%New()
s obj.name=mName, obj.SSN=mKey, obj.Location=mLoc
w "----Great----"
s Status=obj.%Save()
}
catch
{
w "Sorry Unable to insert"
}
znspace ns
quit $$$OK
}

ClassMethod iam(id As %Integer, name As %String, ssn As %String, loc As %String)
{
s mId=id,mName=$zcvt(name,"U")
s ns=$namespace
znspace "USER"
try{
if (##class(Sample.Classes).%ExistsId(mId))
{
s objj=##class(Sample.Classes).%OpenId(mId)
w objj.%Id(),objj.Location
s objj.Location=loc,objj.name=mName,objj.SSN=ssn
s Status=objj.%Save()
w "----Wow Great---------"
}
else
{
w "There is no ",id," is present"
}
}
catch
{
w !,"Sorry Unable to update the Id:",mId
}
znspace ns
q $$$OK
}

ClassMethod there(id As %Integer)
{
S mId=id
s ns=$namespace
znspace "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 ns
q $$$OK
}

ClassMethod now(id As %Integer)
{
S mId=id
s ns=$namespace
znspace "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 ns
q $$$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

Discussion (5)0
Log in or sign up to continue
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 0
Mark Hanson · Mar 3, 2017

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.

1 0