HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Generating a print and a print preview report

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
previewreportgeneratingprintand

Problem

I am developing a report at the moment and I want to apply OOP principle because I feel like my code is not well written and there is duplication.

I have a method that gets data from the database and binds it to the controls on my report.

Here is a sample method I used to print preview a report:

```
public static void CreateReportResult(string poNumber)
{
DataSet ds = new DataSet("PurchaseOrders");
Dal.GetDataSet(ds, sqlQuery(poNumber));
PurchaseReport purchaseOrderReport = new PurchaseReport
{
DataSource = ds,
DataMember = "PurchaseOrders"
};

purchaseOrderReport.xrDateCreated.DataBindings.Add("Text", null, "PurchaseOrders.DateCreated", "{0:dddd, d MMMM, yyyy}");
purchaseOrderReport.xrPO.DataBindings.Add("Text", null, "PurchaseOrders.PONumber", "{0}");
purchaseOrderReport.xrVendor.DataBindings.Add("Text", null, "PurchaseOrders.POVendor", "{0}");
purchaseOrderReport.xrPromocode.DataBindings.Add("Text", null, "PurchaseOrders.PromoCode", "{0}");
//Bill Group
//purchaseOrderReport.xrBusinessName.DataBindings.Add("Text", null, "PurchaseOrders.b_BillToGroup", "{0}");
purchaseOrderReport.xrAddress1.DataBindings.Add("Text", null, "PurchaseOrders.b_AddressLine1", "{0}");
purchaseOrderReport.xrAddress2.DataBindings.Add("Text", null, "PurchaseOrders.b_AddressLine2", "{0}");
purchaseOrderReport.xrCityStateZip.DataBindings.Add("Text", null, "PurchaseOrders.CityStateZip", "{0}");
purchaseOrderReport.xrPhone.DataBindings.Add("Text", null, "PurchaseOrders.b_Phone", "{0}");
//Customer
purchaseOrderReport.xrCustomerBusinessName.DataBindings.Add("Text", null, "PurchaseOrders.CustomerBusinessName", "{0}");
purchaseOrderReport.xrCustomerName.DataBindings.Add("Text", null, "PurchaseOrders.CustomerName", "{0}");
purchaseOrderReport.xrCustomerAddress.DataBindings.Add("Text", null, "PurchaseOrders.customerAddress", "{0}");
purchaseOrderReport.xrCustomerCityStateZip.DataBindings.Add("Text", nu

Solution

Only the most evident flaws ...

  • Very probably your code is vulnerable to SQL injection attacks ...



  • If you want OOP suggestions then i could say that Single Responsibility Principle is violated. Your method CreateReportResult (not even a class, but a method) does a few things - retrieve data from database and create report. So move this logic into DAL "SELECT TOP 1 LiftGate, UPSAccount, LocalPickUp from Purchase_Order where PONumber='" + poNumber + "'");



-
Move duplicated code into separate method (as you mentioned for new requests\improvements you have to change code in 2 methods). Like this

public static void CreateReportResult(string poNumber)
{
    var printTool = GetReportPrintTool();
    printTool.ShowRibbonPreview();   //Preview a Report
}
public static void PrintPDF(string poNumber)
{
    ReportPrintTool printTool = GetReportPrintTool();
    printTool.Print();
}

public static ReportPrintTool GetReportPrintTool()
{
    DataSet ds = new DataSet("PurchaseOrders");
    Dal.GetDataSet(ds, sqlQuery(poNumber));
    PurchaseReport purchaseOrderReport = new PurchaseReport
    {
        DataSource = ds,
        DataMember = "PurchaseOrders"
    };

    purchaseOrderReport.xrDateCreated.DataBindings.Add("Text", null, "PurchaseOrders.DateCreated", "{0:dddd, d MMMM, yyyy}");
    purchaseOrderReport.xrPO.DataBindings.Add("Text", null, "PurchaseOrders.PONumber", "{0}");
    purchaseOrderReport.xrVendor.DataBindings.Add("Text", null, "PurchaseOrders.POVendor", "{0}");
    purchaseOrderReport.xrPromocode.DataBindings.Add("Text", null, "PurchaseOrders.PromoCode", "{0}");
    //Bill Group
    //purchaseOrderReport.xrBusinessName.DataBindings.Add("Text", null, "PurchaseOrders.b_BillToGroup", "{0}");
    purchaseOrderReport.xrAddress1.DataBindings.Add("Text", null, "PurchaseOrders.b_AddressLine1", "{0}");
    purchaseOrderReport.xrAddress2.DataBindings.Add("Text", null, "PurchaseOrders.b_AddressLine2", "{0}");
    purchaseOrderReport.xrCityStateZip.DataBindings.Add("Text", null, "PurchaseOrders.CityStateZip", "{0}");
    purchaseOrderReport.xrPhone.DataBindings.Add("Text", null, "PurchaseOrders.b_Phone", "{0}");
    //Customer
    purchaseOrderReport.xrCustomerBusinessName.DataBindings.Add("Text", null, "PurchaseOrders.CustomerBusinessName", "{0}");
    purchaseOrderReport.xrCustomerName.DataBindings.Add("Text", null, "PurchaseOrders.CustomerName", "{0}");
    purchaseOrderReport.xrCustomerAddress.DataBindings.Add("Text", null, "PurchaseOrders.customerAddress", "{0}");
    purchaseOrderReport.xrCustomerCityStateZip.DataBindings.Add("Text", null, "PurchaseOrders.CustomerCityStateZip", "{0}");
    purchaseOrderReport.xrCustomerPhone.DataBindings.Add("Text", null, "PurchaseOrders.CustomerPhone", "{0}");
    //Items
    purchaseOrderReport.xrTableCell4.DataBindings.Add("Text", null, "PurchaseOrders.ModelNumber", "{0}");
    purchaseOrderReport.xrTableCell5.DataBindings.Add("Text", null, "PurchaseOrders.Quantity", "{0}");
    purchaseOrderReport.xrTableCell8.DataBindings.Add("Text", null, "PurchaseOrders.Price", "{0}");
    purchaseOrderReport.xrTableCell6.DataBindings.Add("Text", null, "PurchaseOrders.Description", "{0}");
    //LiftGate
    //   purchaseOrderReport.xrLiftGate.DataBindings.Add("Text", null, "PurchaseOrders.LiftGate", "{0}");

    DataTable dt = new DataTable();
    Dal.GetDataTable(dt, "SELECT TOP 1  LiftGate, UPSAccount, LocalPickUp from Purchase_Order where  PONumber='" + poNumber + "'");
    foreach (DataRow row in dt.Rows)
    {
        if ((bool)row["UPSAccount"])
        {
            purchaseOrderReport.xrUPS.Visible = true;
        }
        if ((bool)row["LocalPickUp"])
        {
            purchaseOrderReport.xrPickedUp.Visible = true;
        }
        if ((bool)row["LiftGate"])
        {
            purchaseOrderReport.xrLiftGate.Text = "YES";
        }
        else
        {
            purchaseOrderReport.xrLiftGate.Text = "NO";
        }
    }
    ReportPrintTool printTool = new ReportPrintTool(purchaseOrderReport);

    return printTool;
}

Code Snippets

public static void CreateReportResult(string poNumber)
{
    var printTool = GetReportPrintTool();
    printTool.ShowRibbonPreview();   //Preview a Report
}
public static void PrintPDF(string poNumber)
{
    ReportPrintTool printTool = GetReportPrintTool();
    printTool.Print();
}

public static ReportPrintTool GetReportPrintTool()
{
    DataSet ds = new DataSet("PurchaseOrders");
    Dal.GetDataSet(ds, sqlQuery(poNumber));
    PurchaseReport purchaseOrderReport = new PurchaseReport
    {
        DataSource = ds,
        DataMember = "PurchaseOrders"
    };


    purchaseOrderReport.xrDateCreated.DataBindings.Add("Text", null, "PurchaseOrders.DateCreated", "{0:dddd, d MMMM, yyyy}");
    purchaseOrderReport.xrPO.DataBindings.Add("Text", null, "PurchaseOrders.PONumber", "{0}");
    purchaseOrderReport.xrVendor.DataBindings.Add("Text", null, "PurchaseOrders.POVendor", "{0}");
    purchaseOrderReport.xrPromocode.DataBindings.Add("Text", null, "PurchaseOrders.PromoCode", "{0}");
    //Bill Group
    //purchaseOrderReport.xrBusinessName.DataBindings.Add("Text", null, "PurchaseOrders.b_BillToGroup", "{0}");
    purchaseOrderReport.xrAddress1.DataBindings.Add("Text", null, "PurchaseOrders.b_AddressLine1", "{0}");
    purchaseOrderReport.xrAddress2.DataBindings.Add("Text", null, "PurchaseOrders.b_AddressLine2", "{0}");
    purchaseOrderReport.xrCityStateZip.DataBindings.Add("Text", null, "PurchaseOrders.CityStateZip", "{0}");
    purchaseOrderReport.xrPhone.DataBindings.Add("Text", null, "PurchaseOrders.b_Phone", "{0}");
    //Customer
    purchaseOrderReport.xrCustomerBusinessName.DataBindings.Add("Text", null, "PurchaseOrders.CustomerBusinessName", "{0}");
    purchaseOrderReport.xrCustomerName.DataBindings.Add("Text", null, "PurchaseOrders.CustomerName", "{0}");
    purchaseOrderReport.xrCustomerAddress.DataBindings.Add("Text", null, "PurchaseOrders.customerAddress", "{0}");
    purchaseOrderReport.xrCustomerCityStateZip.DataBindings.Add("Text", null, "PurchaseOrders.CustomerCityStateZip", "{0}");
    purchaseOrderReport.xrCustomerPhone.DataBindings.Add("Text", null, "PurchaseOrders.CustomerPhone", "{0}");
    //Items
    purchaseOrderReport.xrTableCell4.DataBindings.Add("Text", null, "PurchaseOrders.ModelNumber", "{0}");
    purchaseOrderReport.xrTableCell5.DataBindings.Add("Text", null, "PurchaseOrders.Quantity", "{0}");
    purchaseOrderReport.xrTableCell8.DataBindings.Add("Text", null, "PurchaseOrders.Price", "{0}");
    purchaseOrderReport.xrTableCell6.DataBindings.Add("Text", null, "PurchaseOrders.Description", "{0}");
    //LiftGate
    //   purchaseOrderReport.xrLiftGate.DataBindings.Add("Text", null, "PurchaseOrders.LiftGate", "{0}");

    DataTable dt = new DataTable();
    Dal.GetDataTable(dt, "SELECT TOP 1  LiftGate, UPSAccount, LocalPickUp from Purchase_Order where  PONumber='" + poNumber + "'");
    foreach (DataRow row in dt.Rows)
    {
        if ((bool)row["UPSAccount"])
        {
            purchaseOrderReport.xrUPS.Visible = true;

Context

StackExchange Code Review Q#139060, answer score: 2

Revisions (0)

No revisions yet.