Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

It looks like you may come from a Java or scripting background. For .NET, the standard recommends PascalCasing for method names. This would turn your methods into:

Render()
GetBrush(...)
GetPoints(...)
GetTension(...)

Variable names should be camelCasing, where the first letter is lower case and the remaining words are capitalized. I'm not sure if NumOfShapes or ParameterNumber are class properties (Pascal Cased) or class-level fields (camel Cased), but you should follow the standard.

As far as your method names, you should avoid GetXXX which suggests that they should be properties. I would recommend the following changes:

GetBrush(...) -> CreateBrush(...)
GetPoints(...) -> GeneratePoints(...)
GetTension(...) -> CalculateTension(...)

For your variable names, curT doesn't help me understand what the parameter is for. :

for (int i = 0; i < NumOfShapes; i++)
{
 int curT = i * ParameterNumber;
 brushes[i] = getBrush(curT); // get value and clip it if need
 points[i] = getPoints(curT); 
 tensions[i] = getTension(curT);
}

The definition and modification must be elsewhere. I'd have to go searching for it. It would be much simpler if you named it something meaningful, even if it is long, like currentImageIteration.

As far as coding goes, always, always dispose of GDI objects when you are done using them. You can get some pretty strange errors (OutOfMemoryException for example) when hitting the GDI object limit. Because GDI uses unmanaged objects, you should always dispose of them when you are done, see this question this question, and this question this question. The object will be disposed at some time in the future, but because the GC doesn't clean up managed objects, this will cause a memory leak.

Other than that, everything seems OK.

It looks like you may come from a Java or scripting background. For .NET, the standard recommends PascalCasing for method names. This would turn your methods into:

Render()
GetBrush(...)
GetPoints(...)
GetTension(...)

Variable names should be camelCasing, where the first letter is lower case and the remaining words are capitalized. I'm not sure if NumOfShapes or ParameterNumber are class properties (Pascal Cased) or class-level fields (camel Cased), but you should follow the standard.

As far as your method names, you should avoid GetXXX which suggests that they should be properties. I would recommend the following changes:

GetBrush(...) -> CreateBrush(...)
GetPoints(...) -> GeneratePoints(...)
GetTension(...) -> CalculateTension(...)

For your variable names, curT doesn't help me understand what the parameter is for. :

for (int i = 0; i < NumOfShapes; i++)
{
 int curT = i * ParameterNumber;
 brushes[i] = getBrush(curT); // get value and clip it if need
 points[i] = getPoints(curT); 
 tensions[i] = getTension(curT);
}

The definition and modification must be elsewhere. I'd have to go searching for it. It would be much simpler if you named it something meaningful, even if it is long, like currentImageIteration.

As far as coding goes, always, always dispose of GDI objects when you are done using them. You can get some pretty strange errors (OutOfMemoryException for example) when hitting the GDI object limit. Because GDI uses unmanaged objects, you should always dispose of them when you are done, see this question, and this question. The object will be disposed at some time in the future, but because the GC doesn't clean up managed objects, this will cause a memory leak.

Other than that, everything seems OK.

It looks like you may come from a Java or scripting background. For .NET, the standard recommends PascalCasing for method names. This would turn your methods into:

Render()
GetBrush(...)
GetPoints(...)
GetTension(...)

Variable names should be camelCasing, where the first letter is lower case and the remaining words are capitalized. I'm not sure if NumOfShapes or ParameterNumber are class properties (Pascal Cased) or class-level fields (camel Cased), but you should follow the standard.

As far as your method names, you should avoid GetXXX which suggests that they should be properties. I would recommend the following changes:

GetBrush(...) -> CreateBrush(...)
GetPoints(...) -> GeneratePoints(...)
GetTension(...) -> CalculateTension(...)

For your variable names, curT doesn't help me understand what the parameter is for. :

for (int i = 0; i < NumOfShapes; i++)
{
 int curT = i * ParameterNumber;
 brushes[i] = getBrush(curT); // get value and clip it if need
 points[i] = getPoints(curT); 
 tensions[i] = getTension(curT);
}

The definition and modification must be elsewhere. I'd have to go searching for it. It would be much simpler if you named it something meaningful, even if it is long, like currentImageIteration.

As far as coding goes, always, always dispose of GDI objects when you are done using them. You can get some pretty strange errors (OutOfMemoryException for example) when hitting the GDI object limit. Because GDI uses unmanaged objects, you should always dispose of them when you are done, see this question, and this question. The object will be disposed at some time in the future, but because the GC doesn't clean up managed objects, this will cause a memory leak.

Other than that, everything seems OK.

deleted 40 characters in body
Source Link
Ron Beyer
  • 1.1k
  • 7
  • 9

It looks like you may come from a Java or scripting background. For .NET, the standard recommends PascalCasing for method names. This would turn your methods into:

Render()
GetBrush(...)
GetPoints(...)
GetTension(...)

Variable names should be camelCasing, where the first letter is lower case and the remaining words are capitalized. I'm not sure if NumOfShapes or ParameterNumber are class properties (Pascal Cased) or class-level fields (camel Cased), but you should follow the standard.

As far as your method names, you should avoid GetXXX which suggests that they should be properties. I would recommend the following changes:

GetBrush(...) -> CreateBrush(...)
GetPoints(...) -> GeneratePoints(...)
GetTension(...) -> CalculateTension(...)

For your variable names, curT doesn't help me understand what the parameter is for. Since curT doesn't change in this loop:

for (int i = 0; i < NumOfShapes; i++)
{
 int curT = i * ParameterNumber;
 brushes[i] = getBrush(curT); // get value and clip it if need
 points[i] = getPoints(curT); 
 tensions[i] = getTension(curT);
}

The definition and modification must be elsewhere. I'd have to go searching for it. It would be much simpler if you named it something meaningful, even if it is long, like currentImageIteration.

As far as coding goes, always, always dispose of GDI objects when you are done using them. You can get some pretty strange errors (OutOfMemoryException for example) when hitting the GDI object limit. Because GDI uses unmanaged objects, you should always dispose of them when you are done, see this question, and this question. The object will be disposed at some time in the future, but because the GC doesn't clean up managed objects, this will cause a memory leak.

Other than that, everything seems OK.

It looks like you may come from a Java or scripting background. For .NET, the standard recommends PascalCasing for method names. This would turn your methods into:

Render()
GetBrush(...)
GetPoints(...)
GetTension(...)

Variable names should be camelCasing, where the first letter is lower case and the remaining words are capitalized. I'm not sure if NumOfShapes or ParameterNumber are class properties (Pascal Cased) or class-level fields (camel Cased), but you should follow the standard.

As far as your method names, you should avoid GetXXX which suggests that they should be properties. I would recommend the following changes:

GetBrush(...) -> CreateBrush(...)
GetPoints(...) -> GeneratePoints(...)
GetTension(...) -> CalculateTension(...)

For your variable names, curT doesn't help me understand what the parameter is for. Since curT doesn't change in this loop:

for (int i = 0; i < NumOfShapes; i++)
{
 int curT = i * ParameterNumber;
 brushes[i] = getBrush(curT); // get value and clip it if need
 points[i] = getPoints(curT); 
 tensions[i] = getTension(curT);
}

The definition and modification must be elsewhere. I'd have to go searching for it. It would be much simpler if you named it something meaningful, even if it is long, like currentImageIteration.

As far as coding goes, always, always dispose of GDI objects when you are done using them. You can get some pretty strange errors (OutOfMemoryException for example) when hitting the GDI object limit. Because GDI uses unmanaged objects, you should always dispose of them when you are done, see this question, and this question. The object will be disposed at some time in the future, but because the GC doesn't clean up managed objects, this will cause a memory leak.

Other than that, everything seems OK.

It looks like you may come from a Java or scripting background. For .NET, the standard recommends PascalCasing for method names. This would turn your methods into:

Render()
GetBrush(...)
GetPoints(...)
GetTension(...)

Variable names should be camelCasing, where the first letter is lower case and the remaining words are capitalized. I'm not sure if NumOfShapes or ParameterNumber are class properties (Pascal Cased) or class-level fields (camel Cased), but you should follow the standard.

As far as your method names, you should avoid GetXXX which suggests that they should be properties. I would recommend the following changes:

GetBrush(...) -> CreateBrush(...)
GetPoints(...) -> GeneratePoints(...)
GetTension(...) -> CalculateTension(...)

For your variable names, curT doesn't help me understand what the parameter is for. :

for (int i = 0; i < NumOfShapes; i++)
{
 int curT = i * ParameterNumber;
 brushes[i] = getBrush(curT); // get value and clip it if need
 points[i] = getPoints(curT); 
 tensions[i] = getTension(curT);
}

The definition and modification must be elsewhere. I'd have to go searching for it. It would be much simpler if you named it something meaningful, even if it is long, like currentImageIteration.

As far as coding goes, always, always dispose of GDI objects when you are done using them. You can get some pretty strange errors (OutOfMemoryException for example) when hitting the GDI object limit. Because GDI uses unmanaged objects, you should always dispose of them when you are done, see this question, and this question. The object will be disposed at some time in the future, but because the GC doesn't clean up managed objects, this will cause a memory leak.

Other than that, everything seems OK.

Source Link
Ron Beyer
  • 1.1k
  • 7
  • 9

It looks like you may come from a Java or scripting background. For .NET, the standard recommends PascalCasing for method names. This would turn your methods into:

Render()
GetBrush(...)
GetPoints(...)
GetTension(...)

Variable names should be camelCasing, where the first letter is lower case and the remaining words are capitalized. I'm not sure if NumOfShapes or ParameterNumber are class properties (Pascal Cased) or class-level fields (camel Cased), but you should follow the standard.

As far as your method names, you should avoid GetXXX which suggests that they should be properties. I would recommend the following changes:

GetBrush(...) -> CreateBrush(...)
GetPoints(...) -> GeneratePoints(...)
GetTension(...) -> CalculateTension(...)

For your variable names, curT doesn't help me understand what the parameter is for. Since curT doesn't change in this loop:

for (int i = 0; i < NumOfShapes; i++)
{
 int curT = i * ParameterNumber;
 brushes[i] = getBrush(curT); // get value and clip it if need
 points[i] = getPoints(curT); 
 tensions[i] = getTension(curT);
}

The definition and modification must be elsewhere. I'd have to go searching for it. It would be much simpler if you named it something meaningful, even if it is long, like currentImageIteration.

As far as coding goes, always, always dispose of GDI objects when you are done using them. You can get some pretty strange errors (OutOfMemoryException for example) when hitting the GDI object limit. Because GDI uses unmanaged objects, you should always dispose of them when you are done, see this question, and this question. The object will be disposed at some time in the future, but because the GC doesn't clean up managed objects, this will cause a memory leak.

Other than that, everything seems OK.

lang-cs

AltStyle によって変換されたページ (->オリジナル) /