Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add files via upload #85

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

tavisca-ssharma
Copy link

@tavisca-ssharma tavisca-ssharma commented Jun 24, 2019

No description provided.

Final result
Copy link

@codeSonic codeSonic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to break the solution into logical units.
Create separate methods having single responsibility so that they are short, readable and easy to maintain.
Try to use .net library methods as and when available instead of re-inventing the wheel e.g. string.Split(), int.TryParse(), string.IndexOf().
Optimize number of iterations and number of if else and nesting in code.

{
int len = equation.length();
char[] val = sentence.ToCharArray();
int i,j=0,k=0,l=0,asum=0,bsum=0,csum=0,fin=0,countr,q,ques,mul,eql;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use meaningful names for variables which convey their purpose

int len = equation.length();
char[] val = sentence.ToCharArray();
int i,j=0,k=0,l=0,asum=0,bsum=0,csum=0,fin=0,countr,q,ques,mul,eql;
for(i=0;i<len;i++)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code is very difficult to follow. lot of for loops and if else conditions in code, everything written in a single method which is more than 100 lines of code.

if(int_result % int_secondnum==0)
{
string temp = (int_result / int_secondnum) + "";
newEquation += temp + "*" + second_num + "=" + result;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why create the equation again. you have got the integer value of missing number by division and you have the string value with ? already, so just take indexof ? and return value at same index from the calculated missing number converted to string


public static int FindDigit(string equation)
{
string first_num = equation.Split('*')[0];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not use snake casing, use camel casing for local variable names e.g. firstNumber

string second_num = equation.Split('*')[1].Split('=')[0];
string result = equation.Split('=')[1];

bool ques_in_first = Int32.TryParse(first_num, out int int_firstnum);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to camelCase

newEquation += first_num + "*" + second_num + "=" + temp;
}

if (equation.Length != newEquation.Length)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all this wont be needed, refer the comment about logic

Tavisca.Bootcamp.LanguageBasics.Exercise1/Assignment1.cs Outdated Show resolved Hide resolved
public static int FindDigit(string equation)
{
string first_num = equation.Split('*')[0];
string second_num = equation.Split('*')[1].Split('=')[0];

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use array of '*' and '=' to split string in a single operation and store in array. this array[0], [1] and [2] will give you a, b and c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants