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

Simple wrapper for LocalReport

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

Problem

I've built a wrapper for a LocalReport and I would like to hear your suggestions about it:

```
public class ReportWrapper : IDisposable
{
private string _reportResource;
private LocalReport _report = new LocalReport();

private int m_currentPageIndex;
private IList m_streams;

public Dictionary ReportDataSource { get; private set; }

public PrinterSettings printSettings { get; set; }

///
/// true, wenn die Seite im Querformat gedruckt werden soll, andernfalls false.
/// Der Standardwert ist false
///
public bool Landscape { get; set; }

public string FileName { get; set; }

public ReportWrapper(string ReportResource)
{
this._reportResource = ReportResource;
this.ReportDataSource = new Dictionary();
this._report = new LocalReport(); ;
}

public void Print()
{
_report.ReportEmbeddedResource = _reportResource;

foreach (var item in ReportDataSource)
_report.DataSources.Add(CreateReportDataSource(item.Key, item.Value));

Export();
Printing();
}

public void toPDF(string FilePath)
{
_report.ReportEmbeddedResource = _reportResource;

foreach (var item in ReportDataSource)
_report.DataSources.Add(CreateReportDataSource(item.Key, item.Value));

var fi = new FileInfo(string.Format(CultureInfo.CurrentCulture, "{0}/{1}", FilePath, FileName));

if (fi.Exists)
fi = CreateNewFileName(fi);

Byte[] results = _report.Render("PDF");
File.WriteAllBytes(fi.FullName, results);
}

private static FileInfo CreateNewFileName(FileInfo fi)
{
int modifier = 1;

while (true)
{
var tempFi = new FileInfo(string.Format(CultureInfo.CurrentCulture, "{0}/{1}({2}){3}", fi.Directory, Path.GetFileNameWithoutExtension(fi.Name), modifier, fi.Extension));

if (!tempFi.Exists)
{
fi = tempFi;

Solution

Naming

Either use m_ or _ or no prefix for your class variables, but don't mix the styles. Also if you use one of the first two, there is no need to use this.

Based on the naming guidelines method names should be named using PascalCase casing. Properties should be named using PascalCase casing too.

Method parameters should be named using camelCase casing.

Using braces {} for single line loop/if statements won't harm the application but makes it less error prone.

This

_report.ReportEmbeddedResource = _reportResource;

foreach (var item in ReportDataSource)
    _report.DataSources.Add(CreateReportDataSource(item.Key, item.Value));


is duplicated code which should be extracted to a method PrepareReport().

Instead of using String.Format() for composing a filename, you should use Path.Combine(). The Path.Combine() method is doing some internal checking like if there are illegal characters in the path etc. which can come handy.

This condition

if (!tempFi.Exists)
{
    fi = tempFi;
    break;
}


can be simplified to

if (!tempFi.Exists)
{
    return tempFi;
}


The input parameters of the CreateStream() method aren't used at all, so you can safely remove them.

Comments should describe why something is done. Let the code speak for itself about what is done by using meaningful names for classes, methods and parameters.

Additional you shouldn't mix languages for commenting. Right now you have german and english comments. I would use english, because if a new developer is hired who doesn't speak german, you almost for sure can say that he/she can speak english.

Because you are calling the Export() method before you are calling the Printing() method you don't need the check for m_streams == null in the Printing() method.

Thinking about this, you don't need the check for m_streams.Count == 0 neither.

Code Snippets

_report.ReportEmbeddedResource = _reportResource;

foreach (var item in ReportDataSource)
    _report.DataSources.Add(CreateReportDataSource(item.Key, item.Value));
if (!tempFi.Exists)
{
    fi = tempFi;
    break;
}
if (!tempFi.Exists)
{
    return tempFi;
}

Context

StackExchange Code Review Q#83710, answer score: 5

Revisions (0)

No revisions yet.