[Mono-devel-list] Patch for System.Data.Common and System.Data.ProviderBase

[Sureshkumar T]

> Index: DbParameterBase.cs
> ===================================================================
> --- DbParameterBase.cs	(revision 44908)
> +++ DbParameterBase.cs	(working copy)
> @@ -4,6 +4,7 @@
>  // Author:
>  //   Sureshkumar T (tsureshkumar at novell.com)
>  //   Tim Coleman (tim at timcoleman.com)
> +//   Boris Kirzner <borisk at mainsoft.com>
>  //
>  // Copyright (C) Tim Coleman, 2003
>  //
> @@ -30,27 +31,30 @@
>  // WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
>  //
>  
> -#if NET_2_0
> +#if NET_2_0 || TARGET_JVM
>  
>  using System.Data.Common;
>  
>  namespace System.Data.ProviderBase {
>  	public abstract class DbParameterBase : DbParameter
>  	{
> +		#region Fields
>  
> -                #region Fields

I guess this is a whitespace changes. 

> -                string _name;
> -                ParameterDirection _direction = ParameterDirection.Input;
> -                bool _isNullable = false;
> +		string _parameterName;

not accepted. mere naming change. 

>  		int _size;
> +#if NET_2_0
>  		byte _precision;
>  		byte _scale;
> -                object _paramValue;
> -                int _offset;
>  		DataRowVersion _sourceVersion;
> +#endif
> +		object _value;

not accepted. mere naming change. 

> +		ParameterDirection _direction;
> +		bool _isNullable;
> +		int _offset;
>  		string _sourceColumn;
> +		DbParameterCollection _parent = null;
>  
> -                #endregion // Fields
> +		#endregion // Fields
>  
>  		#region Constructors
>  	
> @@ -59,11 +63,19 @@
>  		{
>  		}
>  
> -		[MonoTODO]
>  		protected DbParameterBase (DbParameterBase source)
>  		{
> +			if (source == null) {
> +				throw ExceptionHelper.ArgumentNull("source");

this example, where mono's coding guidelines is not followed. refer
bottom of this message.

> +			}
> +
> +			source.CopyTo(this);
> +			ICloneable cloneable = source._value as ICloneable;
> +			if (cloneable != null) 	{
> +				_value = cloneable.Clone();
> +			}
>  		}
> -
> +        
>  		#endregion // Constructors
>  
>  		#region Properties
> @@ -73,9 +85,29 @@
>  			get { throw new NotImplementedException (); }
>  		}
>  
> -                public override ParameterDirection Direction {
> -			get { return _direction; }
> -			set { _direction = value; }
> +		public override ParameterDirection Direction {
> +			get {
> +				if (_direction == ((ParameterDirection) 0)) {
> +						return ParameterDirection.Input;
> +				}

what is this check for? revert to previous default assignment of
ParameterDirection.Input.

> +				return _direction;
> +			}
> +			set {
> +				if (_direction != value) {
> +					switch (value) {
> +							case ParameterDirection.Input:
> +							case ParameterDirection.Output:
> +							case ParameterDirection.InputOutput:
> +							case ParameterDirection.ReturnValue:
> +							{
> +								PropertyChanging();
> +								_direction = value;
> +								return;
> +							}
> +					}
> +					throw ExceptionHelper.InvalidParameterDirection(value);
> +				}
> +			

By the property declaration, this property cannot be set of any value
other than of type ParameterDirection. These exception handling are
irrelevant. Or am I missing something?

> }		}
>  
> +		internal DbParameterCollection Parent
> +		{
> +			get { return _parent; }
> +			set { _parent = value; }
> +		}
> +

what is the need for this parent reference? is the internal type
justified?  Avoid using internal as far as possible as it complicates
the design. Instead, write a internal method wherever you want to access
internal data. i.e. add a public behavior to manipulate a state rather
the state itself.

> +	* DbParameterBase.cs
> +		- Private members names changed to suite naming conventions.

where did you get this naming convetion? I cannot accept changing _name
to _parameterName and terming it as a naming convention. _name inside a
DbParameterBase is always the Parameter's name. I don't see any
advantages of this kind of convention. Please follow whatever defined as
coding guidelines and if you want to add additional coding guidelies,
please propose so that others could follow if good.

it would be nice if you follow mono's coding guidelines, as many places
they are not followed.

> +		- Implemented copy ctor.
> +		- Reimplemented Direction, ParameterName, Size and SourceColumn properties.
> +		- Added #ifdef NET_2_0 on methods not used in NET_1_1

is it the whole purpose? didn't you miss the NET_2_0 spec on top of the
file.

suresh