You are getting these problems, because you are trying to do too much things in one procedure. You should separate your concerns
Separate the fetching of the data from the database from displaying this data; separate also that you do this of event selection changed.
This has the advantage that you can reuse your code more easily: if you want to do the same because of a Button press, you can reuse the code. After that it is a one-liner code if you want to add a menu item doing the same.
It is easier to test your code if you have a separate method to query the database, or a separate method to fill your controls CmbActive and TbSubjectBud.
It is easier to change your code, for instance, if you will not use SQLite anymore, but entity framework to fetch your data, the displaying and the button handling won't notice this. Only the procedure to fetch the data needs to be changed.
This makes unit testing easier: instead of a real database, you can mock the database with a Dictionary for the tests.
Finally: when using Winforms, don't fiddle with the Cells directly, use the DataSource of the DataGridView to fill and read the data. Again: separate the data from how it is displayed.
First Your actual problem: query the data
So you have an Id, and you want to fetch the values of columns Active and Budget from all Subjects from the database that have this Id. Don't fetch properties that you won't use!
Database handling
First we need a class Subject to put the data that you fetch from table SubjectTable. If you put all columns of this table in it, you can reuse the class for other queries. However. you don't have to fill in all fields. It depends how often you will call this method whether it is wise to fill all properties or only some.
Some people don't like this. Consider to fetch always all columns (inefficient), or to create classes for different queries (a lot of work).
class Subject
{
public int Id {set; set;}
public string Name {get; set;}
public DateTime StartDate {get; set;}
public string Active {get; set;}
public Decimal Budget {get; set;}
}
Create a method to fetch the Active and Budget from tableSubjects with Id, or null if there is not subject with this Id.
Put all your database queries in a separate class. For instance class Repository. You hide that it is in a database, if in future you want to save it in a CSV-file, or JSON format, no one will notice (nice if you want to use it in a unit test!)
private Subject FetchBudgetOrDefault(int id)
{
const string sqlText = @"SELECT Active, Budget FROM SubjectTable WHERE ID = @Id";
using (var dbConnection = new SQLiteConnection(this.dbConnectionString))
{
using (var dbCommand = dbConnection.CreateCommand()
{
dbCommand.Commandtext = sqlText;
dbCommand.Parameters.AddWithValue("@Id", id);
using (var dbReader = dbCommand.ExecuteReader())
{
if (dbReader.Read())
{
// There is a Subject with this id:
return new Subject()
{
Id = id,
Active = dbReader.GetString(0),
Budget = (decimal)dbReader.GetInt64(1) / 100.0D,
};
}
else
{
// no subject with this Id
return null;
}
}
}
}
}
I assumed that the decimal Budget is saved as long * 100 on purpose, to show you that by separating your concerns it is fairly easy to change the database layout without having to change all users: if you want to save this decimal in SQLite as a REAL, then the queries are the only place where you have to change the data.
By the way: this method also solved your problem: ID can't be an empty string!
If you won't do this query 1000 times a second, consider to fetch all columns of Subject. This is a bit less efficient, but easier to test, reuse, and maintain.
Display the fetched data in your form
Currently you display the data in a ComboBox and a TextBox. If you separate your concerns, there will only be one place where you do this. If you want to display the data in a Table, or do something else with it, you only have to change one place:
public void Display(Subject subject)
{
this.comboBoxActive.Text = subject.Active;
this.textBoxBudget.Text = subject.Budget.ToString(...);
}
Bonus points: if you want to change the format of the displayed budget, you'll only have to do this here.
Read and Write the DataGridView
It is seldom a good idea to read and write the cells of a DataGridView directly. It is way to much work. You'll have to do all type checking yourself. A lot of work to test and implement small changes in the displayed data.
It is way easier to use the DataSource.
In the DataSource of the DataGridView you put a sequence of similar items. If you only want to Display once, an ICollection<TSource> will be enough (Array, List). If you want to update changes automatically, use a BindingList<TSource>
In the DataGridView add columns. User property DataGridViewColumn.DataPropertyName to indicate which property should be displayed in that column.
Usually it is enough to use visual studio designer to add the columns.
If your datagridview displays Subjects, code will be like:
DataGridView dgv1 = new DataGridView();
DataGridViewColumn columnId = new DataGridViewColumn
{
DataPropertyName = nameof(Subject.Id),
...
};
DataGridView columnName = new DataGridViewColumn
{
DataPropertyName = nameof(Subject.Name),
...
};
... // other columns
dgv.Columns.Add(columnId);
dgv.Columns.Add(columnName);
...
In your forms class:
private BindingList<Subject> DisplayedSubjects {get; set;} = new BindingList<Subject>();
// Constructor:
public MyForm()
{
InitializeComponent();
this.dgv1.DataSource = this.DisplayedSubjects();
}
void FillDataGridView()
{
using (var repository = new Repository())
{
IEnumerable<Subject> fetchedSubjects = repository.FetchAllSubjects();
this.DisplayedSubjects = new BindingList<Subject>(fetchedSubjects.ToList();
}
}
This is all that is needed to display all fetched subjects. If the operator changes any cell value, the corresponding value in this.DislayedSubjects is automatically updated.
This works both ways: if you change any value in this.DisplayedSubjects, the displayed value in the DataGridView is automatically updated.
No need to read the cells directly. If you allow column reordering, or if you implement row sorting, then everything still works two ways. Because you separated the fetched data from the displayed data, you can change the display without having to change the fetched data.
Put it all together
When you get the event that the selection is changed from the datagridview you want to update Active and Budget. Let's do the from the Selected item:
void OnSelectionChanged(object sender, EventHandler e)
{
// Get the selected Subject
var selectedSubject = this.SelectedSubject;
this.Display(selectedSubject); // described above
}
Subject SelectedSubject => this.Dgv.SelectedRows.Cast<DataGridViewRow>()
.Select(row => (Subject)row.DataBoundItem)
.FirstOrDefault();
Because you separated concerns, each method is easy to understance, easy to test, easy to reuse and to change slightly: if you want to update after a Button Press or a menu item, the code will be a one liner. If you want to display other items than just Active / Budget: small changes; if you want to fetch by Name instead of Id: only limited changes needed.
