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

Jinnujohnson #4

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

Conversation

jinnujohnson
Copy link

Indentation corrected in 2 index files of both folders.

@niksmac
Copy link

niksmac commented Sep 1, 2016

Message on Pull Request should explicit.

}
}
private:
int side,column,row;
Copy link
Member

Choose a reason for hiding this comment

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

There should be a new line before the public or private section.

int main()
{
square obj;
obj.square_data();
Copy link
Member

Choose a reason for hiding this comment

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

Use better names; think:

  • Does obj have anything to do with the problem domain?
  • Isn't the function name square_data() redundant for a class named Square?
  • Is a variable assignment must here?

@jikkujose
Copy link
Member

Consider following a pattern while writing commit/PR messages. For instance try to use the verb first like in these:

  • Correct indentation
  • Corrected indentation

Note: capitalization of words

@jinnujohnson
Copy link
Author

Please find 02.cpp file at generalise folder


#include <iostream>
#include "Square.cpp"
Copy link
Member

Choose a reason for hiding this comment

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

There should be a line before using namespace std;

top_bottomSide(draw_variable);
}
};

Copy link
Member

Choose a reason for hiding this comment

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

Remove trailing new line.

@jikkujose
Copy link
Member

jikkujose commented Sep 5, 2016

Avoid accepting input using cin within the Square class. Just like printing the pattern, CLI input isn't the duty of the Square class. Have the data accepted outside & pass the side into the class via constructor.

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.

3 participants