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

q1 and q2 half #4

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

Conversation

Singhal-vishal
Copy link

No description provided.

@abhinav-adtechs abhinav-adtechs added the review pending review pending label Nov 19, 2019
// static TreeMap<String,User> tm = new TreeMap<String,User>();
// static TreeMap<String, ArrayList<User>> tm = new TreeMap<String, ArrayList<User>>();
// static ArrayList<Integer> roll=new ArrayList<>();
public static User getUserInput() {

Choose a reason for hiding this comment

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

Suggest breaking a large function that does too many things into many smaller functions, each doing one thing.

Example: Instead of using getUserInput() where all parameters are input, the function can be divided into inputName() inputAge() inputAddress() inputCourses()

Have a standard way of structuring the verification - example: ideally for Every parameter, there is verification.

So restructuring this function

getUserInput()
-> getName() which internally calls validateNameInput()
-> getCourses() which internally calls validateCoursesInput()
etc.

And a good naming convention will improve Quality of Code and Maintainability.

So each of the above functions should have their own function for Validation. Abstract the utility of functions from each other.

Choose a reason for hiding this comment

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

Here's an example of a more structured approach implementing the above idea.

// Global Variable Messages are a good idea.
final private static String GET_NAME_MESSAGE = "Enter The Username"

User getName(){
System.out.println(GET_NAME_MESSAGE);
input = scan.nextLine();

            if (validateNameInput(input)){
            user.setName(name) ; 
            }
            else { 
            // ERROR LOGIC
            }

}
validateNameInput(){
// VALIDATION CODE
}

System.out.println("Select an option - \n1.Add user\n2.Display user\n3.Delete user\n4.Save user\n5.Exit");
int t=sc.nextInt();
switch(t) {
case 1:

Choose a reason for hiding this comment

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

Using Braces for blocks of code that belong together and Correct Indentation are nice for maintainability even if not required by the language.

}

public static void displayUser() {
List<User> l = dao.findAllUsers();

Choose a reason for hiding this comment

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

always use fully self-explanatory variable names

public class Main extends InventoryUtil{
public static void main(String args[]) {
//calls utility function to start program
start();

Choose a reason for hiding this comment

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

Use OOPs more effectively here -> Create Objects of Classes and call functions as functions of these Objects, instead of using Functions Directly. This allows you to fully utilise the advantages of OOPs. Understand the value of using Singleton pattern cases as well.

//after taking input if item type is wrong then we need to again take input
//that's why above implementation is correct
//public Item calculateTax(Item item) throws InvalidItemException{
public Item calculateTax(Item item){

Choose a reason for hiding this comment

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

Instead of sending the Entire Object as parameter, attempt to use the most primitive possible data types that makes sense to have to reduce the complexity of your functions.

Example: Here we use calculateTax(Item item) { String type=item.getType(); }
Instead, we can use calculateTax(String type) { }
The first method passes an entire object. Ex calling statement: calculateTax(item)
The second only passes the String. Ex calling statement:calculateTax(item.getType())

Many such similar Optimizations can be found.
This also implies that the function is at the most fundamental level in terms of solving a problem.

Item item=null;
String ch="y";
//variable to get out of the loop
int flag2=0;

Choose a reason for hiding this comment

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

Use better variable names that are self documenting so it's clear why they are being used

@@ -0,0 +1,47 @@
package inventory.model;

public class Item {

Choose a reason for hiding this comment

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

Use of explicit Getters, Setters, Default Values are a good idea.

public static Item getItems() {
Item item=new Item();

System.out.println("Enter item details : ");

Choose a reason for hiding this comment

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

Learn to use Command Line Arguments for Console App -> Learn about Shell Input to Java Application

@abhinav-adtechs abhinav-adtechs added review done review done scope for improvement scope for improvement and removed review pending review pending labels Dec 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review done review done scope for improvement scope for improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants