1
\$\begingroup\$

As far as I know, I cannot create function to accept anonymous type as parameter and I am reluctant to create new class to store those anonymous type because their difference is calculatedPrice.

How else can I remove\minimize repeated code in code below?

//.NET Framework 4
protected void btnDownload_Click(object sender, EventArgs e)
{
    var vwTransactionInfoItemProcesses = DAL.TransactionItemProcessHandler.GetVwTransactionInfoItemProcess(dropDownListTransactionStatus.SelectedValue.Trim(), BLL.SessionFacade.currentUserDeptCode.Trim(), true, txtSearchVendorCode.Text.Trim(), txtSearchCreateBy.Text.Trim(), txtSearchTransactionNumber.Text.Trim(), DateTime.Parse(txtCreateFrom.Text.Trim()), DateTime.Parse(txtCreateTo.Text.Trim()).Add(new TimeSpan(23, 59, 59)), txtSearchProductionOrder.Text.Trim(), txtSearchPartNumber.Text.Trim());

    if (BLL.UserRole.CheckUserRoleExist(BLL.SessionFacade.currentUserId, "purchaser"))
    {
        var vwTransactionInfoItemProcessesInDownloadFormat = vwTransactionInfoItemProcesses.Select(x => new
        {
            transactionNumber = x.transactionNumber,
            processName = x.processName,
            processOrder = x.processOrder,
            processQuantity = x.processQuantity,
            processUOM = x.processUOM,
            calculatedPrice = x.calculatedPrice
        }).ToList();
        DataTable transactionInfoItemProcessesInDownloadFormat = BLL.General.ConvertToDatatable(vwTransactionInfoItemProcessesInDownloadFormat);
        HSSFWorkbook workbook = BLL.General.GenerateNPOIExcelXLS(transactionInfoItemProcessesInDownloadFormat);
        ExportToExcel(workbook);
    }
    else
    {
        var vwTransactionInfoItemProcessesInDownloadFormat = vwTransactionInfoItemProcesses.Select(x => new
        {
            transactionNumber = x.transactionNumber,
            processName = x.processName,
            processOrder = x.processOrder,
            processQuantity = x.processQuantity,
            processUOM = x.processUOM
        }).ToList();
        DataTable transactionInfoItemProcessesInDownloadFormat = BLL.General.ConvertToDatatable(vwTransactionInfoItemProcessesInDownloadFormat);
        HSSFWorkbook workbook = BLL.General.GenerateNPOIExcelXLS(transactionInfoItemProcessesInDownloadFormat);
        ExportToExcel(workbook);
    }
}
\$\endgroup\$
2
\$\begingroup\$

Actually you can have anonymous types as parameters - called dynamics - but IMO it's not pretty (and probably rather slow):

static void ConvertToDatatable(IEnumerable<dynamic> input)
{

  foreach (var i in input)
  {
    Type type = i.GetType();
    if (type.GetProperty("Age") != null)
      Console.WriteLine($"{i.Name}: {i.Age}");
    else
      Console.WriteLine($"{i.Name}: No age");
  }
}

static void DynamicTest()
{
  bool isTrue = true;

  IEnumerable<object> data = null;

  if (isTrue)
  {
    data = Enumerable.Range(1, 5).Select(i => new { Name = "Name " + i, Age = i * 10 });
  }
  else
  {
    data = Enumerable.Range(1, 5).Select(i => new { Name = "Name " + i });
  }

  ConvertToDatatable(data);
}

I think I'd use a classical OOP approach:

class BaseClass
{
  public string Name;
}

class Sub1 : BaseClass
{
  public double Age;
}

class Sub2 : BaseClass
{
  public int Age;
}

static void ConvertToDatatable(IEnumerable<BaseClass> input)
{
  foreach (var i in input)
  {
    if (i is Sub1)
      Console.WriteLine($"{i.Name}: {(i as Sub1).Age}");
    else if (i is Sub2)
      Console.WriteLine($"{i.Name}: {(i as Sub2).Age}");
    else
      Console.WriteLine($"{i.Name}: No age");
  }
}

static void BaseClassTest()
{
  bool isTrue = true;

  IEnumerable<BaseClass> data = null;

  if (isTrue)
  {
    data = Enumerable.Range(1, 5).Select(i => new Sub1 { Name = "Name " + i, Age = i * 10 });
  }
  else
  {
    data = Enumerable.Range(1, 5).Select(i => new BaseClass { Name = "Name " + i });
  }

  ConvertToDatatable(data);
}

Beside that, I think your naming is quite remarkable and rather hard to read because of the length:

For instance it's hard to quickly distinguish between:

vwTransactionInfoItemProcessesInDownloadFormat 

transactionInfoItemProcessesInDownloadFormat 

vwTransactionInfoItemProcesses
\$\endgroup\$
2
\$\begingroup\$

You say you are "reluctant" to "create a new class", yet you have not been reluctant to put 6 properties into an anonymous class... I do not understand why you avoid creating a class here, it can basically only benefit you.

In the context you present, that class could look something like this:

class TransactionInformation
{
    int TransactionNumber { get; }
    string ProcessName { get; }
    int ProcessOrder { get; }
    int ProcessQuantity { get; }
    UnitOfMeasurement ProcessUOM { get; }
    decimal? CalculatedPrice { get; }
}

Note that I'm using nullable types here in the assumption that CalculatedPrice is a value type. Also I have omitted the trivial constructor for this class here.

This can be passed easily to any method. ConvertToDatatable now only needs to account for the nullable to not be used in the conversion, if any of your transaction information's CalculatedPrice is null.

Using this datatype the if-block is collapsed into basically nothingness:

var userIsPurchaser = BLL.UserRole.CheckUserRoleExists(...);
var vwTransactionInfoItemProcessesInDownloadFormat = vwTransactionInfoItemProcess.Select(x => new TransactionInformation([..], 
    x.processUOM, userIsPurchaser ? x.calculatedPrice : null)).ToList();

That way the if-block is "hidden" in the construction of TransactionInformation.


Other notes:

  • The use of hungarian notation is highly discouraged. Do not use prefixes like btn, txt, [...]. I think the prefix vw could be useful, but I do not have the slightest clue what it's supposed to mean...
  • The var vwTransactionInfoItemProcess line is too long. A lot of that is just data-cleaning (Trim() anyone?). Your data holders should already contain clean and sanitized data. The problem here is that you have not fully decoupled UI from business logic. You may want to look into patterns like MVVM for that purpose. A stopgap solution would be to introduce a data-holder class an pass that to GetVwTransactionInfoItemProcess instead.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.