65
namespace std
Others have said this already but it's important enough to be repeated: Don't put your own definitions into namespace std. It's undefined behavior.
About the only thing I'm aware of that you may put into namespace std are specializations for templates already defined by the standard. So, for example, if you have
struct MyType
{
int a;
};
...
13
If you're worried about performance, this should raise a red flag:
someJQueryObject = $("#some-id");
...
someJQueryObject.html(aPlainOldString);
.html(foo) is almost certainly slower than .innerHTML = foo, because .html() does a bunch of extra stuff, like checking if it can use innerHTML, trying to use innerHTML, catching errors if that fails and using a ...
11
I've actually implemented the same functionality in the past, so here are my comments.
Wrong behaviour?
It depends on what you can consider wrong. Consider running your own example and inputting a a. The 2nd a will be left in the stream buffer and will be automatically assigned to your 2nd variable. This clearly causes weird behaviour, but it is the same ...
11
namespace std
is reserved. Don't add stuff to it. Especially since there's no reason to in this case. Just put it in your own namespace.
unnecessary string usage
There isn't actually any reason to take the prompt as a std::string. You could have kept it as a character literal. Really, anything streamable is good enough, so this could just be:
template &...
10
When converting time values like this I find it much simpler, and more readable to work up from the smallest units. Consider the following:
// Convert seconds to hh:mm:ss
function secondsToHMS(intime) {
var numseconds = digitize(intime % 60);
intime = Math.floor(intime / 60);
var numminutes = digitize(intime % 60);
intime = Math.floor(intime ...
10
A couple of things:
<Grid Background="{ThemeResource ApplicationPageBackgroundThemeBrush}">
<StackPanel>
...
Putting the StackPanel as single child of the grid is wasteful.
Add RowDefinitions to the grid. As a rule of thumb StackPanel is almost never right. After writing WPF for a couple of years you will probably find yourself ...
10
1.
Instead of:
def __init__(self, root):
"""Constructor"""
self.root = root # root is a passed Tk object
do:
def __init__(self, root: tkinter.Tk) -> None:
self.root = root
Python 3 has built in support for type annotations, so there's no reason to use comments to say what the type of a variable is. If you use mypy (and if you're writing ...
8
Here's a few things I noted:
Don't have using namespace std; in your code. It's almost universally considered a bad practice.
The C++ standard states that you should use int main() and not void main(). Bjarne Stroustrup (a creator of C++) stated that this isn't even C++ (or even C!). I'm surprised you could even get this to work, my compiler won't even ...
8
In playing around with this, I noticed a few things I didn't particularly like:
This picture represents the first problem. Setting the background color fills in the entire rectangle, but should probably only fill in the circle. We can fix this as such:
Add a private variable to hold the value of our circle's background:
private var _backgroundColor: ...
8
Disclaimer
Now that the OP has clarified that they intend to use this as a teaching tool, my answer has lost relevance for their use-case. It can still be taken as general advice for most Python modules.
I do agree with @jpmc26 to a certain extent, specifically that this code itself as currently written should not be used as an example for new learners of ...
7
I (like many others) prefix my jQuery object variables with a $ to make $timeProgress and $timeRemaining. To me, this is a nice explicit reminder that they're jQuery objects, à la Hungarian notation.
duration seems to me like it could be better named totalTime or something.
In digitize(), I see little point in doing "" + n. Use return (n > 9 ? "" : ...
7
This looks good! I've got a couple of comments to make in a vague, hand wavy way so I'll start off with a comment about the code as written:
public SomeMethod(int beforeIndex = -1)
{
if (beforeIndex == -1)
{
No! Bad Mug. No presents for you this Christmas. Use a Nullable<int> and be more idiomatic:
public SomeMethod(int? beforeIndex = null)
...
7
I think your algorithmic part is exceedingly verbose. One of the nicenesses of Python is how it can lead to code that reads almost like English. And for this particular problem, even leaving performance considerations aside, I think iteration is clearer than recursion. So I would rewrite things as:
def digits_of(number):
"""Yields the digits of an ...
7
I'm not sure whether you're reinventing the wheel, but you are using the wrong tools for the job.
I've been through the phase where I think I'm doing something unique enough that I need to do something nonstandard. Every time I've done it, I inevitably regret it. Don't do it. It's not a good idea. Use tools very closely to the way they were intended to be ...
6
You can replace all of the Getbook nested ifs with the following. Place this first:
Dim i As Long
For i = 0 To 6
If Not Getbook("Please select the Book" & i & ".", "Please select a sheet within Book" & i & ".", i) Then
Exit Sub
End If
Next i
Then below that list what you want to happen:
Set Book0 = Workbooks(sListing(0, 0))
...
6
Correctness Issues
main() issues
void main() is not valid C++. main() must return int.
It is also illegal in C++ to call main() from any point in your program. You will need to reorganize your *_Menu() functions to remove the calls to main().
Bug
At the bottom of purchase_order() (in the default: block) you have a single line:
purchase_order;
My ...
6
I think you have the right idea, but your implementation can be improved. I'll give it a try, although I have no clue what rubberduck is, so I might be missing something here. :)
The way you described it, the RubberduckCommand should represent the action to be performed. And it should not matter, whether you invoke this action via menu button, toolbar ...
5
Spaces
Put a space after for and if:
for(int i = 0; i < bound; i++) --> for (int i = 0; i < bound; i++)
if(giveOutput) --> if (giveOutput)
Put a space around a -:
sample.length-1 --> sample.length - 1
And the same goes for +:
System.out.println("Average time taken to sort array: "+totalTime/bound+" nanoseconds");
becomes:
System.out.println("...
5
I don't have the time to do a completely thorough review at the moment, but here are some things that I noticed that may help you improve your code.
Don't use #import
Although it is supported by some compilers, code which is intended to be reused should avoid non-standard extensions. In this case, it's also not really needed because all of the files ...
5
Why not doing it more directly like this:
Private Sub UserForm_Initialize()
Dim i As Byte
For i = 1 To 12
UF_BankRec_cbx_Month.AddItem MonthName(i)
Next
End Sub
Private Sub UF_BankRec_cbx_Month_Change()
Dim i As Byte, startDate As Long
UF_BankRec_cbx_EndDay.Clear
startDate = DateValue("01/" & UF_BankRec_cbx_Month.Text & "/" &...
5
Yes, that is proper use of composition over inheritance. Although some may say this is actually aggregation, due to the nature of Swing.
Composotion is the act of one object owning another object. When Main becomes garbage, so should the objects it's composed of.
Aggregation is a similar concept, except the composed objects do not become garbage. ...
5
Regarding digits_generator
You don't need to use a new variable. You could use nb directly.
You may use the fact that non-0 integers are considered as True in a boolean context and 0 is considered as False to write if nb:.
Anytime you use % and / on the same values, you could use divmod. Here you could write:
def digits_generator2(nb):
while nb:
...
4
Here's what I have so far
The code you wrote doesn't quite work properly; because, depending on the behaviour (which isn't clearly defined) of your HasFocus property:
Either it will return too early (it returns immediately, if this.HasFocus is true when a child has focus)
Or it will return null (it doesn't descend, if child.HasFocus is false when a ...
4
Now that I look at this block mode, I'm finding the logic a bit confusing:
//if the box is already open, use the existing array rather than build a new one
if (!_dwarfJobPermissionBoxIsOpen) {
tempDwarfArray = [_game getDwarvesForRender];
for (DTDwarf *dwarf in tempDwarfArray) {
if (![self isDwarfAlreadyInArray:dwarf]) {
[...
4
First, I'll start by emphatically stating that this method is far too large and tries to accomplish far too much. When your method grows this large, it should definitely be tasked out into smaller methods and functions.
It's okay to write a method or function even if it's only ever called on a single line of code, in a single method, in a single class. If ...
4
I am guessing that the '.volume' elements are like volume indicators, the louder the sound, the more you show.
You can iterate over the '.volume' elements, compare the index / elementCount to ui.value and decide to hide or show per that comparison.
So, something like :
$('.volumeSlider').slider({
value: 1,
orientation: "vertical",
range: "min"...
4
It'd definitely be nice to get rid of the inlined style attributes. Since you've already ID'ed the sub-menu pretty extensively, it's pretty easy to add this to the styles you already have. E.g.
#privacy-menu .glyphicon {
font-size: 10px;
}
gets rid of the inlined font-size styles.
You can also do the toggling by adding a class to the #privacy-menu, ...
4
You can fork the control and do a tiny modification to the code to do what you need.
In touchesEnded:withEvent:, you need to modify this line so it wouldn't check if the tapped segment is the currently selected segment:
if (segment < sectionsCount)
4
All GUI modification must be done in the GUI thread. It refers to the creation of components, too. It means that your buildGUI method should be invoked on the EDT thread(you do it properly for modifications).
This method does not close the input if readLine throws an exception.
try {
File f = new File(loc);
assert f.exists() : "File doesn't exist";
...
4
Table organization
this.menuTable.setPosition(this.stageWidth/10, this.stageHeight/8);
this.menuTable.setWidth((float)(this.stageWidth * 0.8));
this.menuTable.setHeight((float)(this.stageHeight * 0.6));
I have mentioned before that I do not like this. This is not the "correct" way to setup a table on your screen. When you solve this issue, you will no ...
Only top voted, non community-wiki answers of a minimum length are eligible
