Please rate my web service

Solarion

Solarion

Honorary Master
Joined
Nov 14, 2012
Messages
28,059
Reaction score
17,821
Hi guys this is literally my first attempt at this (well many epic failures leading up to this point). I've incorporated some of the other things I've been learning like Unity.

I've also used Basic Authentication. There are two tables in the DB (Students and Logins), Logins for the basic authentication. I've also put a Fiddler help document in the project root. The password and Username is Admin:Admin.

Any sergestions please gentlemen!

Project File: https://github.com/GitRep77/APIWebServiceDemo
 
Last edited:
Why are you using dotnet framework and not dotnet core?
 
_kabal_ said:
Why are you using dotnet framework and not dotnet core?
Click to expand...

I don't have time to learn .NET Core right now. It does have a learning curve and yes I will get there but considering I have absolutely no previous experience with API's I considered .NET Framework is a good place to start and once I have gotten the basics behind me I can go forward with .NET Core.
 
I've fixed it sorry guys :X3:. Was busy reorganizing the whole repo and made that one private by accident.

github.com

GitRep77/APIWebServiceDemo

APIWebServiceDemo. Contribute to GitRep77/APIWebServiceDemo development by creating an account on GitHub.
github.com github.com
 
Just a few general design comments :

API

1. Structure is nice and I see you are leveraging the response codes but for a post, you are creating your own response? Look into the ResponseCreatedAt option.
2. It's never a bad idea to have some xml documentation comments above each method ( parakeets, return types, method descriptions etc). Although your methods are basic CRUD, it's good practice to get into.
3. While you're student api is behind basic authentication, the user is not, so quite a security risk. You can also wrap the whole controller in basic vs each method as you have done, semantics but easy read.

Datalayer

1. In one you use IEnumerable instead of List, I would stick with just list.
2. You can modify your using so that it does the open itself, no need to explicitly state it every time
3. While storeprocs are great, unless you really need to, you can save yourself a lot of code by leveraging dapper ( can't recall if its base or the dapper.contrib.extensions ) but you can do object crud. That saves you have to explicitly define store procedures and parameters. I keep those for db heavy operations. Look into the dapper methods like connection.Insert, .Update . Delete. These simply take a instance of the object as a single parameter.
4. For Bulk operations, which you havent listed, look into leveraging Table Value Parameters in SQL and using dapper to pass lists of those for bulk. There I use procedures and you can also leverage Merge statements.

Model

1. We use the dapper annotation of Key to identify our primary keys to assist the persistence engine.
2. I don't see id fields in your models but your api has id values? I personally like to ensure that the primary key of any table is a simple numeric ID ( one can use GUIDs etc, lots of schools of thought on that ). It makes sense utilizing id fields across queries vs string based fields ( performance )
3. As a design philosophy, I tend to avoid "hard deletes". Although we call it delete in name, it actually runs a update against the table flagging who deleted and when. This keeps good audit and if your record is referenced elsewhere, it doesn't break data integrity by its removal ( yes there are other ways as well but this helps keep a clean audit trail too)

Just a few pointers but hope they help :P @Solarion
 
Last edited:
Thanks mercurial and thank-you for taking the time Kosmik. There is a lot there. I will go through all of it when I get home :thumbsup: :thumbsup:
 
Kosmik said:
2. I don't see id fields in your models but your api has id values? I personally like to ensure that the primary key of any table is a simple numeric ID ( one can use GUIDs etc, lots of schools of thought on that ). It makes sense utilizing id fields across queries vs string based fields ( performance )
Click to expand...

Just on this point here, is it common practice to perform api crud against the primary key of the record?

Example: 1 would be the PK.

C#: 
        // GET: api/Students/1
        [HttpGet]
        [BasicAuthentication]
        public IHttpActionResult GetStudent(int id)
        {
            var student = _repository.GetStudent(id);

            if (student == null)
            {
                return NotFound();
            }

            return Ok(student);
        }
 
Solarion said:
Just on this point here, is it common practice to perform api crud against the primary key of the record?

Example: 1 would be the PK.

C#: 
        // GET: api/Students/1
        [HttpGet]
        [BasicAuthentication]
        public IHttpActionResult GetStudent(int id)
        {
            var student = _repository.GetStudent(id);

            if (student == null)
            {
                return NotFound();
            }

            return Ok(student);
        }
Click to expand...

But there is no id field in your model?


public class Student
{
[Required]
[StringLength(50)]
[System.ComponentModel.DisplayName("Student No")]
public string StudentNo
{
get;
set;
}
[StringLength(50)]
[System.ComponentModel.DisplayName("First Name")]
public string StudentName
{
get;
set;
}
[Required]
[StringLength(50)]
[System.ComponentModel.DisplayName("Last Name")]
public string StudentSurname
{
get;
set;
}
[Required]
[StringLength(1)]
[System.ComponentModel.DisplayName("Gender")]
public string StudentSex
{
get;
set;
}
[StringLength(250)]
[System.ComponentModel.Browsable(false)]
[System.ComponentModel.DisplayName("Address")]
public string StudentAddress
{
get;
set;
}
[Required]
[System.ComponentModel.DisplayName("Date of Birth")]
public DateTime StudentDateOfBirth
{
get;
set;
}
[System.ComponentModel.Browsable(false)]
public byte[] StudentPicture
{
get;
set;
}
[System.ComponentModel.DisplayName("Age")]
public long Age
{
get
{
return Helpers.GetAge(StudentDateOfBirth);
}
}
[Required]
[StringLength(50)]
[System.ComponentModel.Browsable(false)]
[System.ComponentModel.DisplayName("Parent Name")]
public string ParentName
{
get;
set;
}
[Required]
[StringLength(50)]
[System.ComponentModel.Browsable(false)]
[System.ComponentModel.DisplayName("Contact No")]
public string ContactNo
{
get;
set;
}
[StringLength(50)]
[System.ComponentModel.Browsable(false)]
[System.ComponentModel.DisplayName("Email Address")]
public string EmailAddress
{
get;
set;
}
[StringLength(500)]
[System.ComponentModel.Browsable(false)]
[System.ComponentModel.DisplayName("Remarks")]
public string Remarks
{
get;
set;
}
public override string ToString()
{
return StudentName + " " + StudentSurname;
}
}
 
@Solarion did you understand when I spoke about the insert/update methods against objects in dapper?

Dapper Contrib - Learn What Is This Library and How to Use It

Unlock the power of Dapper Contrib by understanding the basics and more advanced topics of this third-party library. Learn what Dapper Contrib is, how it works, and all the documentation you need to get started.
dapper-tutorial.net dapper-tutorial.net
 
Kosmik said:
@Solarion did you understand when I spoke about the insert/update methods against objects in dapper?

Dapper Contrib - Learn What Is This Library and How to Use It

Unlock the power of Dapper Contrib by understanding the basics and more advanced topics of this third-party library. Learn what Dapper Contrib is, how it works, and all the documentation you need to get started.
dapper-tutorial.net dapper-tutorial.net
Click to expand...


I found this. I'll check yours out now. Gotta tell you after using storedprocs for many years, this way makes me feel like, I am wearing shoes without socks. I'll do it and sure I'll get used to it, but it feels off :X3:

www.jeremymorgan.com

Easy CRUD for your Web API with Dapper

A dive into Dapper, a nice simple ORM for .Net. In this tutorial I'll show you how to use Dapper with your ASP.Net Web API
www.jeremymorgan.com www.jeremymorgan.com

C#: 
public bool UpdateCustomer(Customer ourCustomer)
{
  int rowsAffected = this._db.Execute("UPDATE [Customer] SET [CustomerFirstName] = @CustomerFirstName ,[CustomerLastName] = @CustomerLastName, [IsActive] = @IsActive WHERE CustomerID = " + ourCustomer.CustomerID, ourCustomer);

  if (rowsAffected > 0)
  {
      return true;
  }
  return false;
}
 
Not really anything to do with the web service, but one of the things I will judge you mercilessly about is the readme. If you want to look make your life difficult, put a readme in a project that:
1)Is Empty
2) Doesn't contain instructions on how to get the project up and running.


This is a habit everyone should develop. I have literally wasted months on projects where some f__kwit didn't bother writing a note on how to get their software up and running.

So if I were you, I would add the following to your readme:
a) What systems you run it on (OS and version)
b) What dependencies you need to get it running. (any additional libraries and links)
c) What software you used to get the project up and running (what IDE and version)
 
konfab said:
Not really anything to do with the web service, but one of the things I will judge you mercilessly about is the readme. If you want to look make your life difficult, put a readme in a project that:
1)Is Empty
2) Doesn't contain instructions on how to get the project up and running.
3) Keep it updated when you find things that other people should know about your project.

This is a habit everyone should develop. I have literally wasted months on projects where some f__kwit didn't bother writing a note on how to get their software up and running.

So if I were you, I would add the following to your readme:
a) What systems you run it on (OS and version)
b) What dependencies you need to get it running. (any additional libraries and links)
c) What software you used to get the project up and running (what IDE and version)
Click to expand...

I agree with this, even though I am not a developer I like to check out projects and stuff from Git's too, and the readme on some of them is so horrible you can't make heads or tails of anything!
 
R4ziel said:
I agree with this, even though I am not a developer I like to check out projects and stuff from Git's too, and the readme on some of them is so horrible you can't make heads or tails of anything!
Click to expand...
It is a good life skill in general, that whenever you do something complex, you write down what you did.
 
Solarion said:
I found this. I'll check yours out now. Gotta tell you after using storedprocs for many years, this way makes me feel like, I am wearing shoes without socks. I'll do it and sure I'll get used to it, but it feels off :X3:

www.jeremymorgan.com

Easy CRUD for your Web API with Dapper

A dive into Dapper, a nice simple ORM for .Net. In this tutorial I'll show you how to use Dapper with your ASP.Net Web API
www.jeremymorgan.com www.jeremymorgan.com

C#: 
public bool UpdateCustomer(Customer ourCustomer)
{
  int rowsAffected = this._db.Execute("UPDATE [Customer] SET [CustomerFirstName] = @CustomerFirstName ,[CustomerLastName] = @CustomerLastName, [IsActive] = @IsActive WHERE CustomerID = " + ourCustomer.CustomerID, ourCustomer);

  if (rowsAffected > 0)
  {
      return true;
  }
  return false;
}
Click to expand...


Not quite. This is how I would write your same code

Code: 
public bool PutCustomer(Customer customer)
{
    using ( var connection = ConectionHelper.GetOpenConnection(databaseConnetionString))
    {
        return connection.Update<Customer>(customer);
    }
}

Connection is a straight SQLConnection, the helper just checks if there is a open one or creates one. The Update is a dapper.contrib.extension that updates the database by using reflection on the object to build its statement.

github.com

DapperLib/Dapper

Dapper - a simple object mapper for .Net. Contribute to DapperLib/Dapper development by creating an account on GitHub.
github.com github.com

Makes for very neat repo classes.
 
Last edited:
You must log in or register to reply here.
Back
Top
Sign up to the MyBroadband newsletter
X