Trivadis / plsql-cop-cli

db* CODECOP Command Line
Other
24 stars 1 forks source link

Do not report G-7440 when `in out` is used for `self` as part of a fluent API #27

Closed DavidotNet closed 3 months ago

DavidotNet commented 3 months ago

Type OBJECT syntax requires use of IN OUT, (I think) in this case syntax should not be flagged with G-7440: Never use OUT parameters to return values from a function.

create or replace TYPE shape as object ( name varchar2(30 char) , area number

, constructor function shape(self in out nocopy shape , in_name varchar2 , in_area Number) return self as result

, member function Updatee(self in out nocopy shape , in_name in varchar2) return boolean ); / create or replace type body shape as constructor function shape (in_name varchar2 , in_area number) return self as result is begin self.name := in_name; self.area := in_area; return; end;

member function Updatee(self in out nocopy shape , in_name in varchar2) return boolean is begin self.name := in_name; return true; end; end;

PhilippSalvisberg commented 3 months ago

You're correct that self requires to be passed either as in or as in out.

In a constructor function, you have no choice but to use in out. In that case case, we do not throw a G-7440 violation.

However, in other member units, you have a choice. Nobody forces you to use a member function. You could (and should) use a member procedure as in the following amended example of yours:

create or replace type shape as object
(
   name varchar2(30 char),
   area number,

   constructor function shape(self in out nocopy shape,
                              in_name            varchar2,
                              in_area            number) return self as result,

   member procedure updatee(self    in out nocopy shape,
                            in_name in            varchar2)

);
/

create or replace type body shape
as
   constructor function shape(in_name varchar2,
                              in_area number) return self as result is
   begin
      self.name := in_name;
      self.area := in_area;
      return;
   end;

   member procedure updatee(self    in out nocopy shape,
                            in_name in            varchar2)
   is
   begin
      self.name := in_name;
   end;
end;

In this amended example no G-7440 violation is thrown.

DavidotNet commented 3 months ago

An Object contains behavior methods which perform operations on SELF including changes to member attributes; the object state. For both internal (private) methods and external client callers, the caller does not code the Object as a parameter. SELF is implied; it is not physically coded by the external caller as a parameter.

In my Stored Procedure code - the array item 'Name1' is either updated or replaced with new Object state.

Client aa_Associative_Array( Name1 ) := myObject.Updatee( in_value1, invalue2 ); --function result

As suggested, it is my choice how to design the object method signiture. I could pass the Object as a procedure IN OUT parameter. My Stored Procedure code would look like this.

Client object.Updatee( in_value1, in_value2, myObject ); --Procedure method aa_Associative_Array( Name1 ) := myObject;

That would work; however, it is in my opinion, not an elegant coding style as is the first example which more closely resembles C# or Java. The concept of object-oriented programming is different from the usual PLSQL syntax in that, one doesn't pass the object as a parameter but instead writes code using the object dot (.) notation as in:

Object member function Updatee( SELF IN OUT, . . . ) return self as result . . .

Client sys.dbms_output.put_line( aa_Associative_Array( Name1 ).Updatee( in_value1, invalue2 ) ); --The function result is printed. _

Best practices suggested by G-7440 are applicable to normal PLSQL functions. However, as the object type concept and syntax are different (not passing SELF as a parameter by client), so should the application of G-7440 be treated differently in the case of Object types. That's my opinion.

And, thank you for your excellent db CodeCop tool.

PhilippSalvisberg commented 3 months ago

Ok, I see that reporting a G-7440 when building a fluent API is a false positive. In an example like this (returning the object type shape and not a boolean like in your example):

   member function set_name(self in out nocopy shape, in_name in varchar2) return shape is
   begin
      self.name := in_name;
      return self;
   end;

I'll consider changing the validator check to handle this exception. Therefore I move this issue to the CLI repo, where the checks are managed.

PhilippSalvisberg commented 3 months ago

closed with Azure DevOps commit f26d718a87e8bfbf3227a08d4df6dce8610d3740

(G-7440 is not reported for parameters named "self")