kamilo 7 Author Share Posted November 22, 2019 19 hours ago, Nuclear Nezz said: I mean when it's your own code, you can do whatever you want. But if you ever program with another Java developer, you should really get into the habit of using the Java naming conventions. If that's all you're trying to do, then yeah you'd just have 3 simple checks 1. edge case, check if given argument is length 0, if it is return w/e is helpful for your code. 2. if argument length is 1, check parent, return parent info. 3. use getWidgetChild(ids) to get the widget of any depth, whether it's 1 child, a grand child, or a great great grand child, and check the info on that. You have a lot of code that does kinda the same thing. Basically the checks for children and grandchildren in your code is the same thing, and you don't need to do it. It also means you're grabbing the same widgets 3 times by the end. Grab parent first, then parent and child, then parent, child, and grandchild. Not that it means much in the short term, but imagine calling that 100,000 times. Generally when you see chunks of code that do the same thing, but just repeated a few times in a slightly different way, you can refactor it to be less copy-paste code. TYSMM hows this: public boolean nullCheck(int...widgetIds) { // Returns true if widget isn't null, else false boolean check = false; List<Integer> widgetIdList = new ArrayList<Integer>(); for (int widgetId: widgetIds) { widgetIdList.add(widgetId); } if (widgetIdList.size() == 0) { throw new EmptyStackException(); //error } else if (widgetIdList.size() == 1) { Widget parent = ctx.getWidgets().getWidget(widgetIdList.get(0)); if (parent != null && parent.isVisible()) { // => Widget List has only 1 value; therefore checking if parent is null or not check = true; } else { check = false; } } else if (widgetIdList.size() == 2) { WidgetChild child = ctx.getWidgets().getWidgetChild(widgetIdList.get(0), widgetIdList.get(1)); if (child != null && child.isVisible()) { // => Widget has only 2 values; checking if child is null or not check = true; } else { check = false; } } else if (widgetIdList.size() == 3) { WidgetChild grandchild = ctx.getWidgets().getWidgetChild(widgetIdList.get(0), widgetIdList.get(1), widgetIdList.get(2)); if (grandchild != null && grandchild.isVisible()) { // => Widget has 3 values; checking if grandchild is null or not check = true; } else { check = false; } } else { // If more than 3 variables entered, then that doesn't exsist so; false check = false; } return check; } Link to comment Share on other sites More sharing options...
Nuclear Nezz 2040 Share Posted November 22, 2019 @kamilo you're still checking both the 2 and 3 sizes, you don't need to. Instead of doing: if(this thing){ check = true; } else{ check = false; } just do check = this thing; Saves you a couple of lines and looks a little cleaner, imo. I'm not sure why you're moving the ids into the array list you don't need to. It's an array already. if(widgetIds.length == 0){throw exception} else if(widgetIds.length == 1){ check parent } else{ child = getWidgetChild(widgetIds); check child } You don't need to check 2 and 3, because they're the same check, since the method itself just takes an array which you already have. The term int...widgetIds that means that you're given an array in the method, just when you use the method yourself you can do myMethod(1,2,3,4,5,6) instead of having to do myMethod(new int[]{1,2,3,4,5,6}) The arraylist is pointless, and you can't do getWidgetChild(widgetIdList) since they're different data structures. Edit: Sorry, forgot to say: It is looking a lot better, I see more improvements in your code since the original post. Keep going, this is probably a pretty good way for you to learn some of the finer points of programming. Link to comment Share on other sites More sharing options...
kamilo 7 Author Share Posted November 28, 2019 On 11/22/2019 at 4:16 PM, Nuclear Nezz said: @kamilo you're still checking both the 2 and 3 sizes, you don't need to. Instead of doing: if(this thing){ check = true; } else{ check = false; } just do check = this thing; Saves you a couple of lines and looks a little cleaner, imo. I'm not sure why you're moving the ids into the array list you don't need to. It's an array already. if(widgetIds.length == 0){throw exception} else if(widgetIds.length == 1){ check parent } else{ child = getWidgetChild(widgetIds); check child } You don't need to check 2 and 3, because they're the same check, since the method itself just takes an array which you already have. The term int...widgetIds that means that you're given an array in the method, just when you use the method yourself you can do myMethod(1,2,3,4,5,6) instead of having to do myMethod(new int[]{1,2,3,4,5,6}) The arraylist is pointless, and you can't do getWidgetChild(widgetIdList) since they're different data structures. Edit: Sorry, forgot to say: It is looking a lot better, I see more improvements in your code since the original post. Keep going, this is probably a pretty good way for you to learn some of the finer points of programming. Ty sm again, i forgot how to use arrays since first year programming class lmfao superb advice as always heres my end result; /* * Widget Checking = Enter the Widget Id's to check if widget is null or not * i.e. if parent; enter parent widget id, if child; enter parent, child id * OR if grand-child; enter parent, child, grand-child id * Order of ID(s) must be [Parent -> Child -> GrandChild] */ public boolean nullCheck(int...widgetIds) { // True if widget exists (isn't null), else returns false boolean check; if (widgetIds.length == 0) { throw new EmptyStackException(); } else if (widgetIds.length == 1){ Widget parent = ctx.getWidgets().getWidget(widgetIds[0]); if (parent != null && parent.isVisible()) { // => Widget List has only 1 value; therefore checking if parent is null or not check = true; } else { check = false; } } else { WidgetChild child = ctx.getWidgets().getWidgetChild(widgetIds); if (child != null && child.isVisible()) { // => Widget has only 2/3 values; checking if child/grandchild is null or not check = true; } else { check = false; } } return check; } Link to comment Share on other sites More sharing options...
kamilo 7 Author Share Posted November 28, 2019 what about the error choice im throwing? am i using the appropriate one Link to comment Share on other sites More sharing options...
Nuclear Nezz 2040 Share Posted November 28, 2019 1 hour ago, kamilo said: what about the error choice im throwing? am i using the appropriate one I'd probably go with InvalidArgumentException, personally. You could also just return false. You're still doing if(this is true){var = true;} else{var = false;} you can juts do var = checkthis; eg: check = parent != null && parent.isVisible(); Otherwise yeah, looks a lot better. Link to comment Share on other sites More sharing options...
Recommended Posts
Archived
This topic is now archived and is closed to further replies.