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

App to manage a football tournament database

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

Problem

My question is all about class design.

I have this app that is connecting to a SQL server, retrieving some tables, the user works on the data, and then it updates the database.

I should say that all that I know I learned on my own (youtube and reading) but there's still a lot of stuff that makes me confused though.

So for what I have seen, the best approach for my project is to create a class that encapsulates (nice word by the way) this model I just described.

I already have a version working very fine, but the code is all merged into the interface layer. (By working very fine I mean that I can retrieve the data from the database, work on the tables, and then update the database.)

But i fell like I'm achieving this in a very rudimentary way. Now I'm trying to make it more harmonious and well designed. Trying to master my code a bit more I guess.

So here is what I am achieving now based on the "Class" theory and OOP.

Could you help me learn whether this this is good, and if I'm designing my class in the proper way?

```
// Main class that i'm trying to model in such way to be a conceptual model of my real SQL Database (FootballProject Database).
public class fp_Database
{
// Virtual DB.
public static DataSet DS = new DataSet();

// Construtor.
public fp_Database()
{
DS.Tables.Add("Seassons");
DS.Tables.Add("Countries");
DS.Tables.Add("Tournaments");
DS.Tables.Add("TeamsAllowed");
DS.Tables.Add("Matches");

Tables = new fp_Tables();
}

// Public Object fp_Tables containing the fp_Database tables.
public fp_Tables Tables;

// fp_Tables Class definition.
public class fp_Tables
{

// Instantiating all the Databases tables.
public tableMatches Matches = new tableMatches();
public tableSeassons Seassons = new tableSeassons();
public tableCountries Countries = new tableCountries();
public tableTournaments Tournaments = new tableTour

Solution

Class names should be PascalCase and should not contain non-alphanumeric characters, so fp_Database breaks those rules.

tableMatches etc. also break the PascalCase rule.

Methods shouldn't contain non-alphanumeric characters either; Get_Table breaks this rule.

Parameters should be camelCase, so for instance Seasson and Country as used in Set_Filters(int Seasson, int Country) violate this rule.

Local variables like DBInterface as used below should be camelCase:

public tableCountries()
        {
            DatabaseInterface DBInterface = new DatabaseInterface();
            SqlCommand CMD = new SqlCommand("SELECT * FROM fp.Countries");

            DBInterface.GetTable(DS.Tables["Countries"], CMD);
        }


You have a spelling error: "Seassons". However, what's worse is that you repeatedly use this string instead of setting it as a const and using that. Same for other strings.

But the above are merely minor flaws. That they're omni-present is however a warning sign (as well as the obviously copy-pasted parts), and I'm afraid that I don't see any value in this code.

I fear you're relying on very outdated documentation; instead look into ORMs like Entity Framework and how those are used.

Code Snippets

public tableCountries()
        {
            DatabaseInterface DBInterface = new DatabaseInterface();
            SqlCommand CMD = new SqlCommand("SELECT * FROM fp.Countries");

            DBInterface.GetTable(DS.Tables["Countries"], CMD);
        }

Context

StackExchange Code Review Q#157413, answer score: 4

Revisions (0)

No revisions yet.