Skip to content
This repository has been archived by the owner on Feb 27, 2024. It is now read-only.

Konzolos alakzat rajzoló program #4

Open
wants to merge 72 commits into
base: main
Choose a base branch
from

Conversation

Vadgoblin
Copy link

No description provided.

Copy link
Contributor

@webmaster442 webmaster442 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nem gyártottam volna ennyi faja Exception osztályt. Nem ad sokat a funkcióhoz, 1-2 saját exception fajtával bőven le lehetett volna fedni a problémakört. IndexOutofRange exceptiont kézzel meg nem kellene gyártani. A Type discriminator JSON esetén ügyes megoldás.

}
else
{
throw new IndexOutOfRangeException("Index is out of range");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ez egy kicsit redundásns. Ha nem létező indexre hivatkozol, akkor a runtime gyárt belőle IndexOutOfRangeException osztályt és kézzel ilyent nem is lenne szabad gyártani. 🙄

}
else
{
throw new IndexOutOfRangeException("Index is out of range");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Szintén ugyan az.

@@ -0,0 +1,7 @@
namespace E394KZ.Exceptions
{
class CoordinateOutOfCanvas : Exception
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exception osztályból származáskor nem csak ezt a konstruktort kellene definiálni, hanem az összes többit is illene.

@@ -0,0 +1,8 @@
namespace E394KZ.Exceptions
{
class InvalidArgumentumCountException : ShapeException
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exception osztályoknál ajánlás, hogy az Exception legyen publikus, de minimum kiírnám explicit az elérés módosítóját.

@@ -0,0 +1,11 @@
namespace E394KZ.Exceptions
{
abstract class ShapeException : Exception
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Absztrakt osztály egy öröklési hierarcia első eleme ideálisan. Az Exception nem absztrakt, így egy ebből származó osztály absztraktá tételére egy C# programozó se fog számítani. Ez valahol szerintem a Liskov suitability principle-t sérti.


namespace E394KZ.Shapes
{
[JsonDerivedType(typeof(Dot), typeDiscriminator: "dot")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type discriminator ++ 😎

{
if (index < 0 || index >= shapeHistory.Count)
{
throw new IndexOutOfRangeException();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minden egyes kézzel dobott IndexOutOfRangeException miatt valahol meghal egy cuki kiscica. 🐱☠ - ne legyünk gonoszak és szívtelenek.

{
internal record Line : BaseShape
{
public uint EndX { get; init; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A pontok kezelésére lehetett volna egy Point osztály vagy struktúra pl

}
private static bool ContainsInvalidCharacter(string text)
{
string validCharsPattern = @"^[a-zA-Z0-9_.-]+$";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice hogy az eleje és vége is definiálva van, de még egy MatchTimeout-al kiegészíteném a matchelést biztonsági okokból.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants