How to refactor this code for better performance

@tykus

class Person extends Model
{
	public function visits()
    {
        return $this->hasMany(Visit::class);
    }
}

class Visit extends Model
{
	public function person()
    {
        return $this->belongsTo(Person::class);
    }
	public function percentages()
    {
        return $this->hasMany(Percentage::class);
    }
}

class Sickness extends Model
{
	public function percentages()
    {
        return $this->hasMany(Percentage::class);
    }
}

class Percentage extends Model
{
	public function sickness()
    {
        return $this->belongsTo(Sickness::class);
    }
}

@Armani that seems to be only half of the story; how does Person and/or Visit relate to Sickness and/or Percentage?

@tykus sorry I updated above post

@Armani this should work if you want to stay in Eloquent (don't know if you named the table people or persons?)

Person::has('visits.percentages.sickness')->selectRaw('id, count(*) as count')->groupBy('persons.id')->pluck('count', 'id');

@tykus The table is people but still not working as expected: this is out put

2: 1
3: 1
4: 1
4: 1
6: 1

@kokoshneta Actually on every visit a person may have couple of sicknesses and each sickness have percentage. For example on today visit John may have cold for %40, headache %35 and running noses %10.

I want to show a chart with count of people for each sickness.

[Side note: sickness is usually a mass noun in English – it’s the abstract state of being unwell. The actual thing that causes symptoms is called an illness. If other developers have to read your code later on, I would change the class and column names to that for clarity.]

@Armani Ah, so visits are patients’ visits to their doctors, not relatives visiting patients in a hospital? I see how things fit together now.

I would think, then, that each visit needs a direct relationship with an illness, not just with a percentage. Unless you have some other logic you need to perform on the percentages themselves, I don’t think percentages even need to be a model in their own right – they’re simply a property of an illness on a given visit.

If it’s just a number and nothing else, you can have a pivot table connecting visits to illnesses and store the percentage as a third column in that table and fetch that column in your relationships.

Then you’d just do Person::with('visits.illnesses')->get() to retrieve visits including percentages for everyone.

You could also make a hasManyThrough relationship directly from people to illnesses:

class Person extends Model {
	public function visits() {
		return $this->hasMany(Visit::class);
	}

	public function illnesses() {
		return $this->hasManyThrough(Illness::class, Visit::class);
	}
}

With that, you can simply do Person::with('illnesses')->get() to get all the illnesses a person has had.

And the same goes in the other direction:

class Illness extends Model {
	public function visits() {
		return $this->hasMany(Visit::class);
	}

	public function people() {
		return $this->hasManyThrough(Person::class, Visit::class);
	}
}

You should then be able to get the number of people who’ve had a given illness very simply by counting related models:

$illnesses = Illness::withCount('people')->get();

foreach ($illnesses as $illness) {
	echo "Illness {$illness->name} has been suffered by {$illness->people_count} people.";
}

@kokoshneta It's kind of good solution but I don't have sickness_id (illness_id) in visits table. I have it in precentages table. because on each visit a person may have one or more sicknesses.

@Armani Yeah, that was kind of my point – I think your database structure is backwards. A percentage is a property of an individual instance of illnesses, not the other way around.

If you’re working against a production database and can’t change things, that can’t be helped, of course; but if you’re developing from scratch, I would rework the database structure.

The only thing you’d really need to do is rename the table percentages to illness_visit and make sure it has these columns:

┌────┬────────────┬──────────┬────────────┐
│ id │ illness_id │ visit_id │ percentage │
└────┴────────────┴──────────┴────────────┘

That would make it a pivot table connecting illnesses to visits, with the percentage weight of each illness as an additional column.

@kokoshneta well the structure of percentages table is the same you provided.

@Armani Then it’s already a pivot table, not a model data table. What is the purpose of your Percentage model to begin with? What do you do with it?

If you simply rename it to illness_visit and implement the extra relationships I gave above, it should work.

In the visit model, your illness relationship should also be updated to include the extra column:

class Visit extends Model {
	public function person() {
		return $this->belongsTo(Person::class);
	}

	public function illnesses() {
		return $this->belongsToMany(Illness::class)->withPivot('percentage');
	}
}

@kokoshneta I fixed the issue by this query:

$sicknesses = Sickness::has('percentages')->pluck('id', 'name');
		
$personCounts = [];

$sicknessPerPerson = Visit::Join("percentages", "visits.id", "percentages.visit_id")->selectRaw("COUNT(DISTINCT visits.person_id) AS count, sickness_id")->groupBy("percentages.sickness_id")->get();
		
foreach($sicknesses as $sickness){
	$personCounts[] = $sicknessPerPerson->where('sickness_id', $sickness)->first()->count;
}

I know it is better to change pivot table but because there's data inside tables I can't do it right know.

Thanks for all

I know it is better to change pivot table but because there's data inside tables I can't do it right know.

Why not?

If the structure is already the same, as you say it is, the only thing you have to do is rename the table – the data can all stay in place.

@kokoshneta beside data there's other things that makes a difficult to change.