0

I have select2 list with employees which admin will add to project.

I select multiple employees and sending json with employee ID's to controller where trying to get employee by employee, from company database emp list by stored procedure [Employee] to web emp db.

I face such problem that reader reads only first and last rows and skips all employees in midle

    [HttpPost]
    [ValidateAntiForgeryToken]
    public ActionResult EmpAdd()
    {
        var project_id = Convert.ToInt32(Request["project"]);
        var company = Request["company"];
        var emp = Request["emp"];
        var ids = emp.Split(',');
        var project = db.apsk_project.Where(x => x.id == project_id).FirstOrDefault();

        cmd.CommandText = "Employee";
        cmd.CommandType = CommandType.StoredProcedure;
        cmd.Connection = sqlConnection1;

        foreach(var i in ids) {
            var it = Convert.ToInt32(i);
            var kortele = db.apsk_workers.Where(x => x.id == it).FirstOrDefault();
            if(kortele == null) {
                sqlConnection1.Open();

                cmd.Parameters.Clear();
                cmd.Parameters.AddWithValue("company", company);
                cmd.Parameters.AddWithValue("nr", it);
                reader = cmd.ExecuteReader();

                if(reader.HasRows) {
                    while(reader.Read()) {

                        apsk_workers apsk = new apsk_workers();

                        var v = reader.GetString(1);
                        var p = reader.GetString(2);
                        var d = reader.GetDateTime(3);

                        apsk.kof_id = 0;
                        apsk.algos_tipas = "";
                        apsk.vardas = v;
                        apsk.pavarde = p;
                        apsk.ar_dirba = d;
                        apsk.company = company;
                        apsk.manager = User.Identity.Name;
                        apsk.id = it;

                        db.apsk_workers.Add(apsk);
                        db.SaveChanges();
                    }
                }
                sqlConnection1.Close();

                apsk_assigned _Assigned = new apsk_assigned();

                _Assigned.project_id = project_id;
                _Assigned.project = project.project;
                _Assigned.worker_id = it;

                db.apsk_assigned.Add(_Assigned);
                db.SaveChanges();
            } else {
                var ar_projekte = db.apsk_assigned.Where(x => x.project_id == project_id && x.worker_id == it).FirstOrDefault();
                if(ar_projekte == null) {
                    apsk_assigned _Assigned = new apsk_assigned();

                    _Assigned.project_id = project_id;
                    _Assigned.project = project.project;
                    _Assigned.worker_id = it;

                    db.apsk_assigned.Add(_Assigned);
                    db.SaveChanges();
                }
            }
        }

Assigning person to project works fine.

trailmax
  • 34,305
  • 22
  • 140
  • 234
xNaii
  • 111
  • 1
  • 5
  • 3
    ASP.NET MVC is a web framework. It has no readers. Your code seems to mix ADO.NET commands with Entity Framework. Why mix them up? Why not map the stored procedure to an entity? – Panagiotis Kanavos Dec 11 '18 at 13:48
  • 1
    Apart from that, the code seems to open a connection *inside* a loop but close it outside the loop. This will throw in the second iteration. Open the connection just once. You can get rid of the loop if you use `Where(x=>ids.Contains(x.id))`. This will be translated to a `WHERE ID IN (1,4,7...)` clause. You can use the `Id` property of the retrieved objects in the rest of the code – Panagiotis Kanavos Dec 11 '18 at 13:50
  • The open and closed connection places doesn't give any different results, before read loop, I see that I get all lets say 5 employee ID's, but once it comes to reader it keeps only first and last, even thought when calling to procedure it gives back all data. – xNaii Dec 11 '18 at 14:00
  • And I'm doing website not windows form so ado.net has nothing to do here. – xNaii Dec 11 '18 at 14:15
  • 1
    It has everything to do with your code. ADO.NET is the data access library, not the desktop UI. EF works on top of ADO.NET and your code uses ADO.NET classes directly. SqlCommand, SqlConnection, SqlDataReader are all ADO.NET classes. – Panagiotis Kanavos Dec 11 '18 at 14:17
  • You are constructing a single object on the outside of the loop, then you keep updating this inside the loop. This will insert 1 new row the first time and then keep updating it. Move the line ` apsk_workers apsk = new apsk_workers();` inside the loop as well. – Lasse V. Karlsen Dec 11 '18 at 14:22
  • @LasseVågsætherKarlsen invoking model apsk every loop cycle doesn't help. – xNaii Dec 11 '18 at 14:35
  • @PanagiotisKanavos I'm beginner with C# mvc, was googling how to use stored procedures and it was first what I got about data readers and sql connections, plus EF creats sql connections. I'm using asp.net web application (.net framework). What do you suggest for this then ? – xNaii Dec 11 '18 at 14:35
  • Can you edit in how you moved the `apsk` line? – Lasse V. Karlsen Dec 11 '18 at 14:55

1 Answers1

0

I'd make several changes:

  • You are mixing EF with ado.net So please remove all connection opening and closings. Also lines with Parameters etc.
  • Since id's is a string, you can convert it to an List of ints: https://stackoverflow.com/a/9301290/169714
  • With Linq you can assign the employees who are already known to the project
  • With Linq you can foreach add the unknown employees and savechanges after adding them alll.

Let us know if you need help with specific Linq queries. For instance: instead of db.apsk_project.Where(x => x.id == project_id).FirstOrDefault(); you could use db.apsk_project.FirstOrDefault(x => x.id == project_id);

So do not do a foreach and then a first or default to change per row, but think in collections so you can eliminate the loop and increase performance. Hope this helps. If not, please comment so I can try to explain more.

JP Hellemons
  • 5,977
  • 11
  • 63
  • 128
  • Thank you for your answer and help to improve my code. The problem was in form select that returned fired Employees and it returned null here. – xNaii Dec 12 '18 at 11:30