Answer the question
In order to leave comments, you need to log in
Is this structuring of code correct in the context of OOP?
Question for people who understand OOP. I'm getting into OOP on my own (I'm reading the book "Principles, Patterns and Methods of Agile Development in C#" and reworking my old simple console programs, trying to bring them to what I understood as OOP). Due to the fact that I don’t have programming experts in my environment (not to mention OOP), I simply don’t have anyone to ask if I’m doing the right thing and what needs (if necessary) to be corrected. So I decided to dump my works on public display, hoping to get criticism or advice.
First, in general, I have an application that simulates the behavior of sand, I decided to rewrite it by drawing up a UML diagram (which, however, I only finished with the code itself). The diagram looks like this:
the code of all elements is the same:
interface IFallable
{
public void Fall();
public bool CanFall(ParticleStorage ps);
}
abstract class Particle
{
public int x;
public int y;
public char c = '%';
public Particle(int _x, int _y)
{
x = _x;
y = _y;
}
public virtual (int, int) GetPosition()
{
(int, int) Position = (x, y);
return Position;
}
public virtual char GetChar()
{
return c;
}
}
class Sand : Particle, IFallable
{
public Sand(int x, int y) : base(x, y)
{
c = '0';
}
public void Fall()
{
y += 1;
}
public bool CanFall(ParticleStorage ps)
{
if (ps.GetParticleByPosition(x, y + 1) == null)
{
return true;
}
else
{
return false;
}
}
}
class Block : Particle
{
public Block(int x, int y) : base(x, y)
{
c = '#';
}
}
class ParticleStorage
{
List<Particle> particles;
public ParticleStorage(List<Particle> p)
{
particles = p;
}
public List<Particle> GetParticles()
{
return particles;
}
public Particle GetParticleByPosition(int x, int y)
{
Particle returnedParticle = null;
foreach (Particle p in particles)
{
if(p.x == x && p.y == y)
{
returnedParticle = p;
}
}
return returnedParticle;
}
}
class Mover
{
public void MoveAllParticles(ParticleStorage ps)
{
List<Particle> particles = ps.GetParticles();
foreach (Particle p in particles)
{
if(p is IFallable)
{
IFallable f = (IFallable)p;
if (f.CanFall(ps))
{
f.Fall();
}
}
}
}
}
public void DrawAllParticles(ParticleStorage ps)
{
List<Particle> particles = ps.GetParticles();
foreach (Particle p in particles)
{
Console.SetCursorPosition(p.GetPosition().Item1, p.GetPosition().Item2);
Console.Write(p.GetChar());
}
}
}
class Program
{
static void Main()
{
Console.CursorVisible = false;
Drawer d = new();
Mover m = new();
Sand s = new(3, 3);
Block w = new(3, 6);
List<Particle> particles = new List<Particle>();
particles.Add(s);
particles.Add(w);
ParticleStorage ps = new ParticleStorage(particles);
while (true)
{
d.DrawAllParticles(ps);
m.MoveAllParticles(ps);
}
}
}
Answer the question
In order to leave comments, you need to log in
To begin with, I’ll make a reservation that the main programming language for me is F #, which is primarily functional, but I’ll leave a couple of general tips, recommendations and just thoughts, including on OOP.
1. Particle class . Remember - there should not be any public fields in the classes. This is a gross violation of encapsulation. Accordingly, the GetChar method does not make sense, because there is already access to information.
As for the GetPosition method, the existing class heirs do not override it, which means there is no need to make it virtual. In addition, all its uses can be easily replaced with property calls and creating a tuple for such purposes, in my opinion, is superfluous.
abstract class Particle
{
public int X { get; protected set; }
public int Y { get; protected set; }
public char C { get; protected set; } = '%';
public Particle(int _x, int _y)
{
X = _x;
Y = _y;
}
}
abstract class FallableParticle : Particle
{
protected FallableParticle(int _x, int _y) : base(_x, _y)
{
}
public abstract void Fall();
}
if (f.CanFall(ps))
{
f.Fall();
}
public bool Fall();
public bool CanFall { get; }
class Sand : FallableParticle
{
public Sand(int x, int y) : base(x, y) => C = '0';
public override void Fall() => Y += 1;
}
class Block : Particle
{
public Block(int x, int y) : base(x, y) => C = '#';
}
class ParticleStorage : IEnumerable<Particle>
{
readonly ImmutableArray<Particle> particles;
public ParticleStorage(IEnumerable<Particle> p) =>
particles = p.ToImmutableArray();
public bool IsParticleExists(int x, int y) =>
this.Any(p => p.X == x && p.Y == y);
public IEnumerator<Particle> GetEnumerator() =>
((IEnumerable<Particle>)particles).GetEnumerator();
IEnumerator IEnumerable.GetEnumerator() =>
((IEnumerable)particles).GetEnumerator();
}
class Mover
{
public void Move(ParticleStorage ps)
{
foreach (var p in ps.OfType<FallableParticle>())
{
if (!ps.IsParticleExists(p.X, p.Y + 1))
{
p.Fall();
}
}
}
}
code for the sake of code is also a so-so idea, but at first glance everything is ok. some things would be taken out to the mutable constructor, but also a so-so solution.
Code from an OOP point of view is meaningless. Well, in general, too, everything is not very good at all. The review would have to rewrite everything again. But this is not a negative, but simply the first time it is difficult to expect something. Here any algorithm can be easily described in a procedural style, but people use OOP, this gives quite tangible advantages in development and support. What are the benefits of this code?
If you had a personal I would explain in more detail.
Didn't find what you were looking for?
Ask your questionAsk a Question
731 491 924 answers to any question