Skip to content

Comments

Jinnujohnson#4

Open
jinnujohnson wants to merge 18 commits intoKeralaStartupMission:masterfrom
jinnujohnson:jinnujohnson
Open

Jinnujohnson#4
jinnujohnson wants to merge 18 commits intoKeralaStartupMission:masterfrom
jinnujohnson:jinnujohnson

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