Refactor further bitwise functions

Author: fredericDelaporte

src/NHibernate/Dialect/Function/BitwiseFunctionOperation.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections;
+using System.Linq;
 using NHibernate.Engine;
 using NHibernate.SqlCommand;
 using NHibernate.Type;
@@ -15,10 +16,10 @@ public class BitwiseFunctionOperation : ISQLFunction
 		private readonly string _functionName;
 
 		/// <summary>
-		/// Creates an instance of this class using the provided function name
+		/// Creates an instance of this class using the provided function name.
 		/// </summary>
 		/// <param name="functionName">
-		/// The bitwise function name as defined by the SQL-Dialect
+		/// The bitwise function name as defined by the SQL-Dialect.
 		/// </param>
 		public BitwiseFunctionOperation(string functionName)
 		{
@@ -27,45 +28,41 @@ public BitwiseFunctionOperation(string functionName)
 
 		#region ISQLFunction Members
 
+		/// <inheritdoc />
 		public IType ReturnType(IType columnType, IMapping mapping)
 		{
 			return NHibernateUtil.Int64;
 		}
 
-		public bool HasArguments
-		{
-			get { return true; }
-		}
+		/// <inheritdoc />
+		public bool HasArguments => true;
 
-		public bool HasParenthesesIfNoArguments
-		{
-			get { return true; }
-		}
+		/// <inheritdoc />
+		public bool HasParenthesesIfNoArguments => true;
 
+		/// <inheritdoc />
 		public SqlString Render(IList args, ISessionFactoryImplementor factory)
 		{
-			var filteredArgs = new Queue();
-			foreach (var arg in args)
-			{
-				if (!IsParens(arg.ToString()))
-					filteredArgs.Enqueue(arg);
-			}
-
 			var sqlBuffer = new SqlStringBuilder();
 
 			sqlBuffer.Add(_functionName);
 			sqlBuffer.Add("(");
-			while (filteredArgs.Count > 0)
+			var i = 0;
+			foreach (var arg in args)
 			{
-				var arg = filteredArgs.Dequeue();
+				i++;
+				// The actual second argument may be surrounded by parentesis as additional arguments.
+				// They have to be ignored, otherwise it would emit "functionName(firstArg, (, secondArg, ))"
+				if (IsParens(arg.ToString()))
+					continue;
 				if (arg is Parameter || arg is SqlString)
 					sqlBuffer.AddObject(arg);
 				else
 					sqlBuffer.Add(arg.ToString());
-				if (filteredArgs.Count > 0)
-					sqlBuffer.Add(", ");
+				sqlBuffer.Add(", ");
 			}
 
+			sqlBuffer.RemoveAt(sqlBuffer.Count - 1);
 			sqlBuffer.Add(")");
 
 			return sqlBuffer.ToSqlString();

src/NHibernate/Dialect/Function/BitwiseNativeOperation.cs

@@ -13,70 +13,70 @@ namespace NHibernate.Dialect.Function
 	public class BitwiseNativeOperation : ISQLFunction
 	{
 		private readonly string _sqlOpToken;
-		private readonly bool _isNot;
+		private readonly bool _isUnary;
 
 		/// <summary>
-		/// creates an instance using the giving token
+		/// Creates an instance using the giving token.
 		/// </summary>
 		/// <param name="sqlOpToken">
-		/// The operation token
+		/// The operation token.
 		/// </param>
 		/// <remarks>
-		/// Use this constructor only if the token DOES NOT represent a NOT-Operation
+		/// Use this constructor only if the token DOES NOT represent an unary operator.
 		/// </remarks>
 		public BitwiseNativeOperation(string sqlOpToken)
 			: this(sqlOpToken, false)
 		{
 		}
 
 		/// <summary>
-		/// creates an instance using the giving token and the flag indicating a NOT-Operation
+		/// Creates an instance using the giving token and the flag indicating if it is an unary operator.
 		/// </summary>
-		/// <param name="sqlOpToken"></param>
-		/// <param name="isNot"></param>
-		public BitwiseNativeOperation(string sqlOpToken, bool isNot)
+		/// <param name="sqlOpToken">The operation token.</param>
+		/// <param name="isUnary">Whether the operation is unary or not.</param>
+		public BitwiseNativeOperation(string sqlOpToken, bool isUnary)
 		{
 			_sqlOpToken = sqlOpToken;
-			_isNot = isNot;
+			_isUnary = isUnary;
 		}
 
 		#region ISQLFunction Members
 
+		/// <inheritdoc />
 		public IType ReturnType(IType columnType, IMapping mapping)
 		{
 			return NHibernateUtil.Int64;
 		}
 
-		public bool HasArguments
-		{
-			get { return true; }
-		}
+		/// <inheritdoc />
+		public bool HasArguments => true;
 
-		public bool HasParenthesesIfNoArguments
-		{
-			get { return false; }
-		}
+		/// <inheritdoc />
+		public bool HasParenthesesIfNoArguments => false;
 
+		/// <inheritdoc />
 		public SqlString Render(IList args, ISessionFactoryImplementor factory)
 		{
+			if (args.Count == 0)
+				throw new ArgumentException("Function argument list cannot be empty", nameof(args));
+
 			var sqlBuffer = new SqlStringBuilder();
-			var arguments = new Queue(args);
 
-			if (_isNot == false)
-				AddToBuffer(arguments.Dequeue(), sqlBuffer);
+			if (!_isUnary)
+				AddToBuffer(args[0], sqlBuffer);
 
-			AddToBuffer(string.Format(" {0} ", _sqlOpToken), sqlBuffer);
-			while (arguments.Count > 0)
+			sqlBuffer.Add(" ").Add(_sqlOpToken).Add(" ");
+			for (var i = _isUnary ? 0 : 1; i < args.Count; i++)
 			{
-				AddToBuffer(arguments.Dequeue(), sqlBuffer);
+				AddToBuffer(args[i], sqlBuffer);
 			}
 
 			return sqlBuffer.ToSqlString();
 		}
 
 		#endregion
 
-		private void AddToBuffer(object arg, SqlStringBuilder buffer)
+		private static void AddToBuffer(object arg, SqlStringBuilder buffer)
 		{
 			if (arg is Parameter || arg is SqlString)
 				buffer.AddObject(arg);