16

Confusing variable name. Is the tip an amount or percentage? I assumed the former based on variable name, but a commenter has corrected me. Keep calculation and presentation logically separate. You don't need to create a variable for the rounded amount. Just compute the per person cost in the natural way. When you are ready to display that value in a human-...


14

There's a bug in the logic: If I select 0% tip, it is quite possible that adding the rounded amounts each person pays don't add up to the bill total. You should probably use ceil instead of round, to ensure you round up, never down. Of course giving a negative tip percentage also makes you not pay the bill total, but that would clearly be user error. It's ...


10

Get into the habit of localisation instead of hard-coding currency formats and symbols. There's not much of a point to asking for a tip among a list of options if you aren't going to validate it. Even if you were to validate it, I would enforce "between 0 and (perhaps) 100". The tip amount itself isn't used; so just get the total cost directly. Don'...


5

Separate your concerns Split input parsing from your calculation logic. Read and parse the input in one step and do the calculation in a separate function. Use fitting data types. Using float as a data type can have nasty side-effects. Consider using decimal.Decimal instead. Also the three parameters to calculate the bill belong together, since they are only ...


2

Yes, this code is not easy and I don't have an easy answer for you. Only a couple tips. The problem is not complexity, the problem is that the code is too long and overwhelming. First of all, you have too many nested control blocks but you already know that. For instance the opening starts at line 4: if (kryptonDataGridView2.SelectedRows.Count > 0) { ...


2

It’s an adventure game, why not seeing the code also from an adventurous perspective. Let’s imagine a character that traverses a sequence of scenes that each has: a story: With your newly discovered {element} powers, you mysteriously wake up in a whole new world. You're in a crowded forest and there's noises coming from every direction. A small furry animal ...


1

Review I have gone through all the code, this is a part of a single function that seems to be handling an event in the user interface, possibly a selection event on a data grid. There are 450 lines of code and that is way too much for a single function. A general rule in current programming is that any function larger than one screen in an editor is too ...


1

Fundamentally, you are trying to keep a row if: It meets some sorts of conditions that make it a "good" row, or It's the first row for an id that has no "good" rows. To me, then, the way to accomplish this would be to compute which rows are good rows and to filter accordingly. It might look something like this: filtered2 <- inputDF %&...


1

Unit tests It's great that you have included unit tests for these functions. That makes reviewing and proposing modifications much easier than otherwise. Thank you! Naming It's normal to write header files for inclusion with a name ending in .h. MathLibrary.cpp is likely to be mistaken for the top level of a translation unit (aka an implementation file). ...


Only top voted, non community-wiki answers of a minimum length are eligible