Interesting question,

I understand you search for a rails-style way on how to write nodejs programs. However from a CodeReview perspective, your code should be idiomatic (conforming to the mode of expression characteristic of a language) so that other developers can grok your code and even maintain it.

From that perspective your code is quite good, the only thing being that you really (really) should put those semi-colons. Just get over it, good JavaScript has semicolons at the end of each line.

Other than that:

I like the use of 'use strict', generator functions and yield
I like the amount of comments, not so much the style, I would go for less vertical real estate

/*
 * User controller.
 */

could just as well be

//User controller

It's a matter of taste but I find this more idiomatic

var skip = Math.max(0, this.query.skip),
    limit = Math.min(10, this.query.limit);

than

var skip, limit
skip = Math.max(0, this.query.skip)
limit = Math.min(10, this.query.limit)

I guess from a style perspective you want to split declaration from initialization, but I find this takes too much vertical real estate.
Logging/Handling errors, you should consider putting more effort in handling errors than simply throwing a 403 on the client side, you should do something on the server side as well
Magic constants (404, 200) etc. are known well enough that you don't have to create named constants.

All in all I like your code, I guess if I had to maintain I would run it thru a script to fix all the semicolons and comments and go from there ;)


Adding a custom field to Routes - clever solution or terrible hack?

I'm not sure if my solution is relevant to your problem, but I had a similar problem with keeping the correct page selected on page refresh as refresh was resetting defaults and home would be selected on what ever page you refreshed from.

This is how I solved it:

page controllers

app.controller('aboutCtrl', ['$rootScope', function($rootScope){
    $rootScope.$broadcast('pageSelected', "about");
}]);

app.controller('stuffCtrl', ['$rootScope', function($rootScope){
    $rootScope.$broadcast('pageSelected', "stuff");
}]);

app.controller('homeCtrl', ['$rootScope', function($rootScope){
    $rootScope.$broadcast('pageSelected', "home");
}]);

When the page controller is initiated, it broadcasts pageSelected with the name of the page as the value (important to use $rootScope to avoid parent/child cases).

navigation menu controller

app.controller('headerCtrl', ['$scope', '$rootScope', function($scope, $rootScope){
    $scope.$on('pageSelected', function(e, val){
        $scope.selected = val;
    });
}]);

The menu controller is listening for the pageSelected broadcast and will set $scope.selected to the value received.

navigation menu html

Home

To Do

Stuff

About

Finally, the menu will update which item has the selected class dynamically based on $scope.selected.

In your case, you could maybe have the page controllers that need the same menu item selected to broadcast the same value?

Although not necessary, I also recommend using ui-router.  It is state-based instead of URL-based; that's why I have ui-sref="home" ect.


Single word CAS tagged pointer for algorithms susceptible to the ABA problem

It's just a spin lock. If the thread that invalidated of the tag got suspended right after, no other thread can make progress till the thread is resumed and releases the tag.

if (old_tag & INVALIDATED) {
    return false; // no thread can go further if ...
}
if (tag.compare_exchange_strong(old_tag, old_tag | INVALIDATED,
                                std::memory_order_acquire)) {
    ptr = newptr; // <-- ... a thread is preempted here
    tag.store(old_tag + 2, std::memory_order_release);
    return true;
}

So the solution is not lock-free.


This somewhat depends on which principle you value higher. Small repetition:

F.I.R.S.T

F: Fast

Pretty self-explaining. Unit tests have to be as fast as possible for TDD to be viable.

I: Independent

Unit tests are not depending on anything else than the testing-framework and the class under test (cut). Not even on other tests. Unit tests prepare their own data and setup and clean up after themselves after evaluation. It must not matter in which order unit tests are executed!

R: Repeatable

Unit tests give the same result when they are repeatedly executed. Even on a new moon with low tide. They always look for the same results and perform the same operations.

S: Self-Validating

Unit tests validate the result of any operation themselves. They may not give any results to external classes for validation. Also they should be self-explaining as to why that result is expected.

T: Timely

Unit tests are to be written as close as possible before the implementation of a feature

principles freely adapted from Agile in a Flash

In this case you will have to evaluate the fast principle against the independent principle
It will definitely be faster to create the instance of ComplexClass only once (on fixture setup), but that will undermine the independency of unit tests and may will cause problems when you depend on execution order.


Rate limit a method by generating particular load on demand in C#

Welcome to CR.  There is a lot of room for improvement with your code.  Let's start with the simple stuff and work our way up.

CR prefers use of braces always with if or for or foreach.

Hungarian Notation is heavily frowned upon, so get rid of names prefaced with m_.

Underscores internal to variable names are also frowned upon, so sleep_time would be better as sleepTime (or better altogether with a different name: see further below).

Names should meaningful and create clarity rather than confusion.  So m_threads isn't the number of threads running, but rather its the number of tasks you will start.  Thus that variable name should have "tasks" in it rather than "threads".

You should place your code into its own class.  For instance, class ThroughputTester.  There would be class level properties or fields named TaskCount, TargetThroughput, and TossesPerSecond.  I would not have these as constant fields, since it makes your code rigid and only applicable to the one set of values.  Rather these could be readonly properties set in the constructor, which allows for experimentation with different sets of values.

You could also have a CreateDefault method to create an instance with the same values as your constants.

public class ThroughputTester
{
    public ThroughputTester(int taskCount, int targetThroughput, int tossesPerSecond)
    {
        TaskCount = taskCount;
        TargetThroughput = targetThroughput;
        TossesPerSecond = tossesPerSecond;
    }

    public int TaskCount { get; }
    public int TargetThroughput { get; }
    public int TossesPerSecond { get; }

    public static ThroughputTester CreateDefault => new ThroughputTester(100, 2000, 2000);

}

Your use of Random is fragile and runs into issues, particularly since you have it in a method with a loop.  The better usage would be to define the Random instance at the class level instead of inside a method.

sleep_time leaves me wondering what it represents.  Is it seconds or milliseconds?  Can't tell from a vague name like time.  It could be renamed sleepMilliseconds.  Alternatively, you could use the Thread.Sleep(TimeSpan) signature (see link here) along with the TimeSpan.FromMilliseconds or TimeSpan.FromSeconds methods.

That said, even Thread.Sleep has some negativity associated with it.  You should read this answer for Why is Thread.Sleep so harmful.


Do less writing

You can half the code by using the fact \$x^{-n} = \frac{1}{x^n}\$

if n > 0:
    while n > 0:
    ...
else:
    while n < 0:
    ...

would become

if n < 0:
    # x**(-n) == 1 / x**n
    return 1 / old_code(x, -n)

return old_code(x, n)

(with old code being the code you have from the while loop down)

Likewise \$(-x)^n = x^n\$ if n is even, otherwise \$-(x^n)\$. This extra check can be done at the start rather than in the middle of a loop. Combining the two you get

if n < 0:
    power = -n
else:
    power = n

if x < 0:
    base = -x
else:
    base = x

result = old_code(base, power)

if base != x and power % 2:
    # (-x)**n == -(x**n) if n is odd, otherwise x**n
    result = -result

if power != n:
    # x**(-n) == 1 / x**n
    result = 1 / result

return result

This probably could bit a little nicer, but hopefully it is very clear what is happening. From this you can write your code assuming both the base and power are positive.

Better algorithm

We can improve the algorithm from O(n) to O(log n), where n is the power, using the idea from russian peasant exponentiation. Alternatively you could implement exponentiation by squaring for the same runtime complexity. We can derive the idea by the following observation

$$x^n = \overbrace{x \times x \times x \times \cdots \times x}^{n}$$

$$x^n = \overbrace{x \times x \times x \times \cdots \times x}^{n/2} \times \overbrace{x \times x \times x \times \cdots \times x}^{n/2}$$

$$x^n = y \times y, y = \overbrace{x \times x \times x \times \cdots \times x}^{n/2}$$

We need to make a slight adjustment if n is odd, as n/2 wont be an integer

$$x^n = x \times \overbrace{x \times x \times x \times \cdots \times x}^{\lfloor n/2 \rfloor} \times \overbrace{x \times x \times x \times \cdots \times x}^{\lfloor n/2 \rfloor}$$

$$x^n = x \times y \times y, y = \overbrace{x \times x \times x \times \cdots \times x}^{\lfloor n/2 \rfloor}$$

We can then work out \$x^{n/2}\$ recursively until some easy to compute base case.

Note: To turn this into a loop we can do the same thing, but starting at lower powers and working up. See the pdf for how to continue.

All in all my suggested code would look something like this

def pow(x, n):
    if n == 0:
        return 1
    if x == 1:
        # New easy check
        return 1
    if x == 0:
        if n < 0:
            # builtin pow throws an exception rather than returning None
            raise ZeroDivisionError("...")
        return 0

    if n < 0:
        power = -n
    else:
        power = n

    if x < 0:
        base = -x
    else:
        base = x

    result = _pow(base, power)

    if base != x and power % 2:
        # (-x)**n == -(x**n) if n is odd, otherwise x**n
        result = -result

    if power != n:
        # x**(-n) == 1 / x**n
        result = 1 / result

    return result

def _pow(base, power):
    """Return base**power
    Assume base > 0, power > 0"""
    if power == 1:
        return base

    y = _pow(base, power // 2)
    result = y * y
    if power % 2:
        result *= base
    return result


How can I improve this code without using static or singleton?

I believe that some generalizations are to be made lightly. i.e. stating that Singletons are evil in general is not accurate, as there are some cases where Singleton is a proper (and maybe the only) solution. And yes I do believe that it is one of the overly abused  patterns out there. Having gotten that out of the way, I move to answer your question.

From the code sample you submitted, it is very clear that your CustomerBLL has no business logic and merely acts as a Façade on top of the data access and other helper classes. Facades should be stateless by nature.

Having said that, I will offer two choices based on testability and decoupling requirements.

If you want to make you CustomerBLL testable and decouple it from the CustomerHelper and CustomerDAL concretions and substitute them with abstractions, then one good design enhancement would be to inject the CustomerHelper and CustomerDAL dependencies into the CustomerBLL class, which allows you to mock and stub the data access and helper classes and focus on testing other code modules that consume the CustomerBLL class. You acheive that through extraxcting interfaces of course.

Here is an example of how the code would look if we inject the dependencies through a constructor, note that your CustomerBLL Façade will be an instance class i.e. no static methods will be exposed.

public class CustomerBLL
{
    private readonly ICustomerHelper _helper;
    private readonly ICustomerDAL _dal;

    // default constructor initializes the concrete dependencies
    public CustomerBLL()
        :this(new CustomerHelper(), new CustomerDAL())
    {}

    // constructor to inject stubbed dependencies for testing purposes
    public CustomerBLL(ICustomerHelper customerHelper, ICustomerDAL customerDal)
    {
        _helper = customerHelper;
        _dal = customerDal;
    }

    public void Method1(string param)
    {
        _dal.Method1(param);
    }

    public void Method2(string param)
    {
        _dal.Method2(param);
    }

    public void MethodA(int param)
    {
        _helper.MethodA(param);
    }
}

public interface ICustomerDAL
{
    void Method1(string s);
    void Method2(string s);
}

public interface ICustomerHelper
{
    void MethodA(int i);
}

internal class CustomerDAL : ICustomerDAL
{
    public void Method1(string s)
    {
    }

    public void Method2(string s)
    {
    }
}

internal class CustomerHelper : ICustomerHelper
{
    public void MethodA(int i)
    {
    }
}

If on the other hand, testability is not a concern at all, then you can proceed by consuming the CustomerBLL as a Façade in a procedural way through static methods (although I much prefer the testability approach, but it’s not every one’s cup of tea).

Here you notice I removed the private fields that hold references to the Helper and DAL classes, because we want to promote that the CustomerBLL Façade is stateless.

public class CustomerFacade
{
    public static void Method1(string param)
    {
        new CustomerDAL().Method1(param);
    }

    public static void Method2(string param)
    {
        new CustomerDAL().Method2(param);
    }

    public static void MethodA(int param)
    {
        new CustomerHelper().MethodA(param);
    }
}

I have seen and used both approaches, in my humble opinion they both are valid depending on the usage scenario. In general I am in favor of testable design and decoupled modules and that’s why I proposed the first approach.


Generating the property name of an object

Yes, this is bad practice, sometimes

You are giving the user the option to dynamically change object properties. What if the property doesn't exist? What if a boogie monster passes by?

Sometimes, generating the property could be usefull, but only do this when you trust the thing that is generating the property. Is it a user? bad practice. Is it the program itself, wel, maybe.

escaping data

Don't escape data, don't try and remove all 'bad' data. A better approach is to check for a whitelist.

$role has to be an integer, so make sure it is

<?php

if ( preg_match('/^[0-9]+$/',$_POST['role']) != 1 )
{
    throw new Exception('I need an int');
}

feature and permission probably only accept alpha characters? And probably only lower case? So let's check that:

if ( preg_match('/^[0-9a-z]+$/',$_POST['role']) != 1 )
{
    throw new Exception('I need an int');
}

whitelists are better

Use whitelists instead of blacklist. It's easier to know what is allowed then to know what isn't. Simply because we can't know all the bad things out there


Queries for the number of users seen each day

You're right, you should avoid querying in a loop as much as you can. If $dayCount were 30, I would run a query like:

SELECT
    DATEDIFF(CURRENT_DATE,FROM_UNIXTIME(last_seen)) as inactive_days,
    COUNT(*) AS num_rows
FROM example.servers
WHERE UNIX_TIMESTAMP(DATE_SUB(CURDATE(),INTERVAL 30 DAY)) <= last_seen
GROUP BY inactive_days
ORDER BY inactive_days ASC

This should give you an array of rows, each row would have inactive_days holding the number of days between today and last seen day, and num_rows holding the number of records that were grouped into that row. The result set is also ordered with the fewest inactive days first. Your PHP script could then use that result as needed.  If you fetch the array and call it $result, you could do

foreach($result as $row):
    echo "$row[num_rows] users where last seen $row[inactive_days] days ago";
endforeach;

Quick notes:

Be sure to test! I may have a bug in my code, but you get the gist. If you find a bug, leave a comment asking me to fix it.
Don't do anything you don't understand.  Take the time to learn what each line of code does, so you can get better, catch errors and even improve upon it.
If the order of the results doesn't matter to your logic, don't use the ORDER BY clause; sorting is computationally expensive and should only be used when needed


Comparison of incoming error strings to known set and transformation into 3rd string

The inner for loops can be made easier:

match1 = max(fuzz.token_set_ratio(error, internal_error) for internal_error in error_list1)

And similar for match2 and match3.

However, the real complixity in your implementation lies in the determining of the error message to show.

I would suggest building a dictionary mapping the display error to internal errors:

def interpret_error(error):
    error_mapping = {
        "User error 1": [
            "Failure: 'mean_va' is not in list",
            "Failure: No JSON object could be decoded",
            "Failure: Expecting ':' delimiter: line 1 column 14410098 (char 14410097)",
            "Failure: cannot concatenate 'str' and 'ContentDecodingError' objects",
            "Failure: Unknown error",
            "Failure: Error returned by EPA WATERS web service: No Results Returned",
        ],
        "User error 2": [
            "Failure: Error returned by EPA WATERS web service: Flowline comid 4505568 at measure 79.99388 not found in NHDPlus stream network",
            "Failure: Error returned by EPA WATERS web service: No Catchments match navigation results",
        ],
        "User error 3": [
            "Failure: Error returned by EPA WATERS web service: Start permid must have a hydroseq greater than the hydroseq for stop permid",
            "Failure: Error returned by EPA WATERS web service: Start measure must be greater than stop measure when navigating a single reach",
        ],
    }

    display_error = None
    score = 0.0
    for message, internal_errors in error_mapping.items():
        tmp = max(fuzz.token_set_ratio(error, internal_error) for internal_error in internal_errors)

        # This matches better than any earlier internal_error.
        if tmp >= score:
            score = tmp
            display_error = message

    return display_error

Ideally: move the mapping to a separate .py file, or some form of config file (.json, .yaml or even .xml), or the database.


I'm not surprised at your results, bar a few minor details that any decent database will optimize away your implementations are the same. You are also basically just re-implementing the find_each method.

Note, you might find a real difference if you used database cursors but that is a completely different thing.

The real difference between the two is that cursor based pagination responds better when the underlying data changes. i.e. using offset based pagination you might have a record included twice or skipped if a record is inserted or deleted by another process.


I don't have much time for the review so I'll focus on regular expressions for the time being.

About re

You should probably be using match instead of search because you want the pattern to be at the beginning of the string. Read more
If you are re-using the same regexp multiple times like you are doing here, it might be worth compile-ing it.
You could use the logic used for matching to split your result as well. If you do so, you do not need to change things in multiple places if the format changes;

Like this :

>>> date = "14/07/1789"
>>> m = re.match(r'(\d{1,2})/(\d{1,2})/(\d{4})', date)
>>> m.groups()
('14', '07', '1789')

Even better, you could use named elements. If the format changes (because your code is used in a different country where dates have a different format for instance), only the regexp will change, the code will still be ok because it will be accessing elements by their name and not by their position.

Like this :

>>> date = "14/07/1789"
>>> m = re.match(r'(?P<day>\d{1,2})/(?P<month>\d{1,2})/(?P<year>\d{4})', date)
>>> m.groups()
('14', '07', '1789')
>>> m.groupdict()
{'month': '07', 'day': '14', 'year': '1789'}

Also, the whole logic converting strings to dates (or to triples (day, month, year)) could be extracted in a function on its own.

You might also decide that you want to reuse already existing tools for this such as datetime.strptime

Better user experience

When I input 2 dates which is usually quite painful, I'd rather have my program to fail if the first date is incorrect rather than having to input the second date to know it.


Why is a method that is ostensibly about "copying file dependencies" killing a process?

It also seems awkward to use recursion for something like this, especially from the catch block. Maybe it's just me?

I would get rid of the recursion and the process killing, and let the calling code higher up deal with that. This method should just attempt to copy the files from a to b, and the calling code can run it as many times as it wants, handle failures by killing a process or whatever, etc.


Multithread the variant of matrix multiplication

Let's start with the simplification changes:

Declare variables and fields as final wherever possible. This drastically simplifies reasoning about mutation behaviour in your code. Since matrixA and matrixB are not supposed to change for the lifetime of the Object, they should be declared final.
Make use of caching when it makes sense: Currently your code will always recompute the result if the call method is executed, regardless of whether it has already been or not. That's basically a waste of computing power you could prevent by just computing the result once and allowing future calls to just lookup that result.
Keep the scope of variables as small as possible. Double check that introducing a variable actually makes sense before doing that. The code currently declares almost all variables it uses upfront. That reminds me of old vba programs. In either case: it's easier for you (and the runtime) when variables are in the smallest scope possible

future, start and end can be declared inside the respective loop body. in call you only use n, m and k to decide whether you splitAndMerge or run the calculation on a single thread. Those variables are basically just fluff. Inline them to prevent confusion when reading the code.
You seem to have an aversion to whitespace. It's hard to subidivide the code into logical sections because there's no distinction between blocks. This makes grasping the code significantly harder.

There's a large possibility for simplification in your design. Currently the code attempts to break the problem into "subresult" computations by "tasks". The problem I see with this is that matrices make this harder than it needs to be. To compute the result at \$(i,j)\$ you need the left hand sides \$i\$th row and the right hand sides \$j\$th column.
Since it's a bit unclear to me how the matrices are organized in your code, I'll not get into the details, but I agree with oopexpert when he comments:

  If that is the case and there is nothing special about the matrix multiplication I would follow a totally other approach. I would forget about any thread handling and formulate the problem in a way that I am able to use parallel streams

Now to make myself sufficiently clear: I'm not proposing you reformulate this as a solution using Streams. I haven't quite found a way to make that look nice. Instead I'd strive to calculate each entry independently. This will allow you to trivially control how much CPU-power you want to throw at the problem (instead of this somewhat messy solution).

I think you could have a good chance at reformulating this using a simplification. Since you only read from the input matrices and result matrix entries are fully independent of one another you can have great success enqueueing a separate Task for each entry that task only needs matrixA, matrixB as well as a row and column to perform it's duties. You can control the level of parallelism by setting a limit on how many Threads your executor pool may use.

Overall that solution strikes me as a heavy simplification over the current code, because it doesn't rely on subdividing the matrices into submatrices. That makes it much more understandable: one task for one result.


Determine whether a vector intersects the up or down unit vector

You have four groups of repeating conditions:

bool crossX = (lastDiff.x() < 0 && diff.x() > 0) || (lastDiff.x() > 0 && diff.x() < 0);
bool crossZ = (lastDiff.z() < 0 && diff.z() > 0) || (lastDiff.z() > 0 && diff.z() < 0);
bool crossTouchX = (lastDiff.x() <= 0 && diff.x() >= 0) || (lastDiff.x() >= 0 && diff.x() <= 0);
bool crossTouchZ = (lastDiff.z() <= 0 && diff.z() >= 0) || (lastDiff.z() >= 0 && diff.z() <= 0);

Using these the original condition can be simplified to:

if ((crossX && crossTouchZ) || (crossZ && crossTouchX))
{
    // do stuff
}


React component to animate planets on a canvas

You can do this, if it works... but it's not really using the React architecture or patterns.

Most of this code that is drawing should be in the render method.
You'll want to move some of the state (canvas, ctx, bgCanvas, bgCtx) into the class's state, or just as instance variables.
You can probably do most of the creation in the constructor, although doing it in componentDidMount will work.
You can integrate the animation with React,
setInterval(() => this.forceUpdate(() => this.t += 0.05)). This makes sense in componentDidMount. You'll also want to turn this off in the unmount method.

EDIT/ADDED LATER

...maybe you just want to leave it the way it is, although it's quite non-React... it's working fine, though, and there aren't really problems...

To make it "React" you're probably going to have to re-write it completely-- because just moving pieces around like I suggested may make you end up fighting the framework more than leveraging. Looking more, you'd probably make separate components for the different canvases and perhaps pass in t as a property, so there's a container component that functions as the "clock". Doing something like that would allow the regular React rendering mechanisms work.


Number of Occurrences of a word in a text file or group of text files

While the functional approach may not always be the best solution, I find using Streams on files very convenient.

If you need to extend the second requirement on directories even further to include all .txt files in subdirectories, you can easily recurse through and flatmap all of the subdirectories until you are left with just a single stream of .txt files.

Code:

public static void main(String[] args) throws IOException {

    Scanner inputReader = new Scanner(System.in);

    System.out.println("Enter a path");
    String inputPath = inputReader.nextLine();
    System.out.println("Enter the word");
    String targetWord = inputReader.nextLine();

    File file = new File(inputPath);

    long numOccurances = 0;

    if(file.isFile() && file.getName().endsWith(".txt")){

        numOccurances = toLineStreamFromFile(file)
                            .flatMap(str -> Arrays.stream(str.split("\\s")))
                            .filter(str -> str.equals(targetWord))
                            .count();

    }else if(file.isDirectory()){

        numOccurances = Arrays.stream(file.listFiles(pathname -> pathname.toString().endsWith(".txt")))
                              .flatMap(Main::toLineStreamFromFile)
                              .flatMap(str -> Arrays.stream(str.split("\\s")))
                              .filter(str -> str.equals(targetWord))
                              .count();
    }

    System.out.println(numOccurances);
}

public static Stream<String> toLineStreamFromFile(File file){
    try {
        return Files.lines(file.toPath());
    } catch (IOException e) {
        e.printStackTrace();
    }
    return Stream.empty();
}


Automated Java fighting and battling simulation

Hi thanks for sharing your code!

Players class

Creating a class to represent a Player is a great idea, however your current class represents both players. If you want to have 2 players in your game, it might make more sense to have a Player class rather than a Players class.

Also try and expose the bare minimum that is possible by making as much private as possible. Don't reveal implementation details, only expose what is absolutely required.

Instantiating your players could look like this.

int hp = 100;
Player playerOne = new Player(playerName, hp);
Player playerTwo = new Player("Opponent", hp);

Where possible avoid using classes as a way of storing global / static variables. The benefit of creating classes is to provide a method of abstraction.

On top of this Player class your Game class is essentially just some static methods that you call from the main method. What if your code could look like this

Game game = new Game(player1, player2);
game.fight();

I re-wrote a version of your program, hopefully you find this useful.

Main

import java.util.Scanner;

public class Main {

    public static void main(String[] args) {
        Scanner scan = new Scanner(System.in);

        System.out.println("Please enter your username.");
        String userName = scan.nextLine();

        System.out.println("Please enter initial hp.");
        int hp = scan.nextInt();

        Player human = new Player(userName, hp);
        Player opponent = new Player("Opponent", hp);

        Game game = new Game(human, opponent);
        game.fight();
    }
}

Player

import java.util.Random;

public class Player {
    private final String userName;
    private int hp;

    public Player(String userName, int initialHp) {
        this.userName = userName;
        hp = initialHp;
    }

    public void attack(Player enemy) {
        Random rnd = new Random();
        int damage = rnd.nextInt(21) + 1;
        enemy.dealDamage(damage);
        System.out.println(getUserName() + " dealt " + damage + " to " + enemy.getUserName());
        System.out.println(enemy.getUserName() + " has " + enemy.getHp() + " hit points left!");
    }

    public void dealDamage(int damage) {
        hp -= damage;
        hp = hp < 0 ? 0 : hp;
    }

    public boolean isDead() {
        return hp <= 0;
    }

    public String getUserName() {
        return userName;
    }

    public int getHp() {
        return hp;
    }
}

Game

import java.util.Random;

public class Game {

    private final Player humanPlayer;
    private final Player aiPlayer;

    public Game(Player humanPlayer, Player aiPlayer) {
        this.humanPlayer = humanPlayer;
        this.aiPlayer = aiPlayer;
    }

    public void fight() {
        try {
            Thread.sleep(1000);
        } catch (InterruptedException ex) {
            Thread.currentThread().interrupt();
            System.out.println("INTERUPTED DELAY");
        }

        while (true) {
            humanPlayer.attack(aiPlayer);
            if (aiPlayer.isDead()) {
                System.out.println("Opponent is dead!");
                victory();
                return;
            }

            aiPlayer.attack(humanPlayer);
            if (aiPlayer.isDead()) {
                System.out.println("You are dead!");
                defeat();
                return;
            }
        }
    }

    private void victory() {
        System.out.println("You are the ultimate warrior.");
        System.out.println("Thanks for fighting..........");
    }

    private void defeat() {
        System.out.println("You have been slain.");
        System.out.println("Thanks for fighting..........");
    }

}

Some changes of note and general advice

Added some logic to the Player class where I thought it would be reasonable to do so. These should hopefully make the code more readable too.

The return statement in the method when either player dies will exit immediately.

Made it so each player has a different random number. Otherwise here the Player would win every time since they hit first.

I would recommend making values final if possible. E.g. the userName or the Random object.

Don't blindly make getters and setters for everything (this is a commonly encouraged practice especially when you're first learning to code), make getters only if you need to and setters if you really need to mutate the value.

Don't make member variables public. This makes it very hard to change your code. Use getters if you need to access the values.

small thing here, the line if (Players.oHP == 0 || Players.oHP < 0) could simply be replaced with if(Players.oHP <= 0)

A class should be considered a blue print for making an object, not a way of accessing all information about all the objects.

Hopefully this review was helpful for you, keep it up!


Middleware for sending PartialViews via email

Implementing Nkosi's suggestions was a great step forward. This way the client can also recieve the response. However, there was one more thing that could be greatly improved. While reading about Queued background tasks I realized that this is exactly what I need to speed up my middleware becasue sending emails made it hang for a short time. Queueing the task in a background service allows the middleware to return immediately and take its time for emails.

I adjusted the example code for my needs by not using a logger here. Instead I placed the Debug.Fail inside the catch clause (and changed a couple of names).

public interface IWorkItemQueue
{
    void Enqueue(Func<CancellationToken, Task> workItem);

    Task<Func<CancellationToken, Task>> DequeueAsync(CancellationToken cancellationToken);
}

public class WorkItemQueue : IWorkItemQueue
{
    private readonly ConcurrentQueue<Func<CancellationToken, Task>> _workItemQueue = new ConcurrentQueue<Func<CancellationToken, Task>>();

    private readonly SemaphoreSlim _signal = new SemaphoreSlim(0);

    public void Enqueue(Func<CancellationToken, Task> workItem)
    {
        if (workItem == null) { throw new ArgumentNullException(nameof(workItem)); }

        _workItemQueue.Enqueue(workItem);
        _signal.Release();
    }

    public async Task<Func<CancellationToken, Task>> DequeueAsync(CancellationToken cancellationToken)
    {
        await _signal.WaitAsync(cancellationToken);
        _workItemQueue.TryDequeue(out var workItem);

        return workItem;
    }
}

public class WorkItemQueueService : IHostedService
{
    private readonly IWorkItemQueue _workItemQueue;

    private readonly CancellationTokenSource _shutdown = new CancellationTokenSource();

    private Task _backgroundTask;

    public WorkItemQueueService(IWorkItemQueue workItemQueue)
    {
        _workItemQueue = workItemQueue;
    }

    #region IHostedService

    public Task StartAsync(CancellationToken cancellationToken)
    {
        // ReSharper disable once MethodSupportsCancellation - this task is not supposted to be cancelled until shutdown
        _backgroundTask = Task.Run(BackgroundProceessing);

        return Task.CompletedTask;
    }

    public Task StopAsync(CancellationToken cancellationToken)
    {
        _shutdown.Cancel();
        return Task.WhenAny(_backgroundTask, Task.Delay(Timeout.Infinite, cancellationToken));
    }

    #endregion

    public void Enqueue(Func<CancellationToken, Task> workItem)
    {
        _workItemQueue.Enqueue(workItem);
    }

    private async Task BackgroundProceessing()
    {
        while (!_shutdown.IsCancellationRequested)
        {
            var workItem = await _workItemQueue.DequeueAsync(_shutdown.Token);

            try
            {
                await workItem(_shutdown.Token);
            }
            catch (Exception)
            {
                Debug.Fail("Work item should handle its own exceptions.");
            }
        }
    }
}

The updated MailerMiddleware now enqueues tasks sending emails on that queue so that the service can handle them later and also handles its own exceptions.

public class MailerMiddleware
{
    private readonly RequestDelegate _next;

    private readonly ILogger _logger;

    private readonly IWorkItemQueue _workItemQueue;

    private readonly IEmailClient _emailClient;

    public MailerMiddleware(RequestDelegate next, ILoggerFactory loggerFactory, IWorkItemQueue workItemQueue, IEmailClient emailClient)
    {
        _next = next;
        _logger = loggerFactory.CreateLogger<MailerMiddleware>();
        _workItemQueue = workItemQueue;
        _emailClient = emailClient;
    }

    public async Task Invoke(HttpContext context)
    {
        var bodyBackup = context.Response.Body;

        using (var memory = new MemoryStream())
        {
            context.Response.Body = memory;

            await _next(context);

            memory.Seek(0, SeekOrigin.Begin);
            using (var reader = new StreamReader(memory))
            {
                var body = await reader.ReadToEndAsync();

                var email = context.Email();
                if (!(email.To is null || email.Subject is null))
                {
                    _workItemQueue.Enqueue(async cancellationToken =>
                    {
                        try
                        {
                            await _emailClient.SendAsync(new Email<EmailSubject, EmailBody>
                            {
                                To = email.To,
                                Subject = new PlainTextSubject(email.Subject),
                                Body = new ParialViewEmailBody(body),
                            });
                        }
                        catch (Exception ex)
                        {
                            _logger.Log(Abstraction.Layer.Network().Action().Failed(nameof(IEmailClient.SendAsync)), ex);
                        }
                    });
                }

                // Restore Response.Body
                memory.Seek(0, SeekOrigin.Begin);
                await memory.CopyToAsync(bodyBackup);
                context.Response.Body = bodyBackup;
            }
        }
    }

    private class ParialViewEmailBody : EmailBody
    {
        private readonly string _body;

        public ParialViewEmailBody(string body)
        {
            _body = body;
            IsHtml = true;
            Encoding = System.Text.Encoding.UTF8;
        }
        public override string ToString()
        {
            return _body;
        }
    }
}

ASP.NET-Core is really cool ;-)


//Search between 2 and 1000
public static final int minNum = 2;
public static final int maxNum = 1000;

That comment doesn't say anything that the code doesn't already state IMO. Now if you need to change either value, you have to update the comment as well, otherwise you end up with this:

//Search between 2 and 1000
public static final int minNum = 2;
public static final int maxNum = 10000;

And would that be a typo in the code, or just a misleading comment?

for (int num=minNum; num <= maxNum; num++)

Spacing isn't consistent between operators, that's distracting when you read it.

for (int num = minNum; num <= maxNum; num++)

Reads more fluently. Now, you have a scope, but where's the opening brace? Code that's explicit won't omit them, even for a single statement.

if (perfectNum(num)) {

Might be I'm the verbose type, but I don't see a reason not to rename num to number or value, and perfectNum to isPerfectNumber, making the condition read like this:

if (isPerfectNumber(value)) {

Now, I like how minNum and maxNum are consistent with num, so I'd then rename them to minValue and maxValue to maintain that beautiful consistency.

int i_sumDiv = 0;

Now that's a bit of a mystery. why is there an i_ prefix? The Div abbreviation seems uncalled for, and makes the name a bit ambiguous.

Okay I need to stop the line-by-line analysis here.

You're doing the work twice. First here:

if (perfectNum(num)) {

and then if the function returns true, you do the work all over again - you know it's a perfect number already; you're ready to output right there and then:

if (isPerfectNumber(value)) {
    System.out.println(num + " is a perfect number.");
}

So let's look at that function.

public static Boolean perfectNum(int num) {
    // a variable that holds the sum of the factors
    int sum = 0;

    for (int factor = 1; factor < num; factor++) {
        if (num % factor == 0) {
            sum += factor;
        }
    }

    if (sum == num) {
        return true;
    } else {
        return false;
    }
  }
}

I like factor much more than j. I'm seeing braces on all scopes, and although sum isn't a completely descriptive noun, it's much better than i_sumDiv: I like this implementation much better than the superfluous one that executes after this function returns.

The indentation is broken though:

    }
  }
}

Should be:

        }
    }
}

Also, this is redundant:

    if (sum == num) {
        return true;
    } else {
        return false;
    }

It can easily be replaced with a single return statement:

return sum == num;

for (int factor = 1; factor < num; factor++) {

That loop is inefficient; the (redundant) one in main is better.


Remove specified number of characters from a string

One quick stylistic comment.

Braces

for ( ; i < index + rm_length; ++i )
    if ( text[i] == '\0' ) {
        text[index] = '\0';
        return;
    }

Constructs like this are begging to have a subtle hidden bug in them. Always use braces if you have more than a single statement inside a for/if statement.

for (; i < index + rm_length; ++i) {
    if ( text[i] == '\0' ) {
        text[index] = '\0';
        return;
    }
}


using Regex.Replace with a help of PadLeft would do this in a single line. so your work can be simplified to :

foreach (var file in Directory.GetFiles(path, "*.epub", SearchOption.AllDirectories))
{
    var fileName = System.IO.Path.GetFileName(file);
    var filePath = System.IO.Path.GetDirectoryName(file);//gets the path sans the  file
    var strName  = Regex.Replace(fileName, @"^((\d+\.){1}(\d\.)?\s+)", m => m.Groups[0].Value.Trim().PadLeft(4, '0').PadRight(8));

    File.Move(file, Path.Combine(filePath, strName));
}

The regex should covers any string with the same pattern in your provided sample.

UPDATE :

To make it more readable for you, you can use the following :

var regex = new Regex(@"^([0-9]+\.?\.\s)");

var fileName = new StringBuilder();

foreach (var file in Directory.GetFiles(path, "*.epub", SearchOption.AllDirectories))
{
    fileName.Append(Path.GetFileName(file));
    var filePath = System.IO.Path.GetDirectoryName(file);

    var match = regex.Match(fileName.ToString());

    if (match.Success)
    {
        fileName.Clear();
        fileName
            .Append(match.Value.Trim().PadLeft(4, '0'))
            .Append("    ")
            .Append(file.Substring(match.Value.Length).Trim());
    }

    fileName.Replace(fileName.ToString(), Path.Combine(Path.GetDirectoryName(file), fileName.ToString()));

    File.Move(filePath, fileName.ToString());

    fileName.Clear();
}

Using StringBuilder is a must in your case, since you're dealing with a list of files name, and since string is immutable, using StringBuilder will avoid creating a new string for each filename which would save your memory and also optimize the performance.


Formatting strings

You should try to avoid those kind of things:

$statement = $args['dbType'].':'.'host='.$args['host'].';'.'dbname='.$args['dbName'];

It's easy to miss something, and it's also this kind of constructions that leads to difficult to read code and (in other contexts) SQL injections. I would argue that using sprintf is a better idea here:

$format = '%s:host=%s:dbname=%s';
echo sprintf($format, $args['dbType'], $args['host'], $args['dbName']);

I don't know if it is considered idiomatic PHP or not, but it seems more readable and less error-prone to me.

PDO and exceptions

mnhg already gave you a way to get rid of the first return NULL in PDOConnection_Factory::build(), but it's not enough, you should also remove the second one. I assume it comes from the possible exceptions in PDO::__construct and PDO::setAttribute. The issue is that you're catching the exception but doing nothing with them.

You should instead embrace exceptions. :) A good first step is the setAttribute call. Since there's nothing you can do to handle them in the factory: you should instead let the application code decide what to do if something fails: for example, display a warning message to users and send an email to the administrator.


Painting a lot of diagrams for a board game

Instead of rendering the bitmap in memory, you could define a custom view that takes a Board and renders it.

I did this for an Android app for a variant of the Go board game, and it worked pretty well. The basic idea of this AbstractBoardView is to have the board state somewhere and to connect it to a view so that it can be drawn.

The memory requirements for this class are exceptionally low, compared to a precomputed bitmap. Even more because my implementation doesn't even hold the board state, it just provides access to it via abstract methods.


Since you said you were looking for suggestions for coding style I will focus on that.

Comment your code! Preferably with Haddock style comments. Even if you aren't planning on sharing your code with others, having comments will still help you if you decide to come back to a project after a break.

Haddock is a system that will automatically generate documentation based on your comments. Formatting your comments to work with Haddock basically just mean adding a | as the first character of your top level comments. For example:

-- | Pretty prints a Sudoku board encoded as a list.
printGrid :: [Char]
printGrid [] = ...

All top level symbols should have type signatures. Types in Haskell are super expressive and often convey as much meaning or more than variable names.
Line wrap your code at 80 or 100 characters (80 seems to be more popular). Not everybody has as wide a monitor as you, and limiting the length of your lines forces you to break up code into logical units.
Haskell programs tend to have lots of short variable names. This makes sense for general functions like map where you can't really be specific about the values even if you wanted to:

map f (x:xs) = f x : map f xs

However, the more specific your code gets the less applicable (and more confusing) short variable names get. For example, the argument to all of your functions is x. This a missed opportunity to give the reader more information on what the parameter of this function actual is.
You are using a String to represent your Sudoku board. This is fine, but it makes your type signatures unhelpful. You can fix this by defining an alias for your board type:

-- | An entry in a Sudoku board is either the number as a
-- character, or '0' to indicate an unknown value.
type Entry = Char

-- | A Sudoku board is a list of Sudoku entries read from
-- left to right, top to bottom.
type Board = [Entry]

You have several take n/drop n and takeWhile/dropWhile pairs in your code. You can combine these into a single statement with splitAt and span.

front = take 9 x
back  = drop 9 x
(front, back) = splitAt 9 x

front = takeWhile (/= '0') x
back  = dropWhile (/= '0') x
(front, back) = span (/= '0') x

You have a lot of if-then-else blocks in your code. Often times these can be more succinctly expressed with pattern guards. For example, you can simplify your solve function as follows:

-- | Takes a board with possibly unknown entries and returns
-- Just a board with all of the entries containing valid numbers,
-- or Nothing if there is no solution.
solve :: Board -> Maybe Board
solve board | isSolved board = Just board
            | otherwise      = listToMaybe solutions
    where solutions = filter isSolved $ map solve (candidates board)

Also, your solve method returns the empty list if no solution can be found. This is confusing because the empty list is also a valid board. When your function has the possibility of failing, it is best to wrap the result in a Maybe or another type that is capable of representing an error.
Your candidates function is doing a lot of list traversals with your use of ++ and takeWhile and dropWhile. You can get rid of most of these traversals by using recursion to find the first element of the list and replacing it inline.

-- | Generate a list of possible solutions to a Sudoku board by
-- replacing the first unknown entry with all possible values.
candidates board = filter isValidCandidate [ replaceFirst guess board
                                           | guess <- ['1'..'9']]

-- | Replace the first unknown entry in a Sudoku board with
-- the given entry.
replaceFirst :: Entry -> Board -> Board
replaceFirst _ []       = []
replaceFirst a ('0':xs) = a : xs
replaceFirst a (x:xs)   = x : replaceFirst a xs

Another note about candidates,  you can incorporate the filter as part of the comprehension you are already doing:

candidates board = [ candidate | guess <- ['1'..'9']
                               , candidate <- [replace guess board]
                               , isValidCandidate candidate]

I stared at the isInvalidCand function for a while and I'm still having trouble groking all that is happening. Some comments and some better naming would go a long way in improving the readability this function.


INSERTing a student info record based on form inputs

Like I had mentioned in my comment this is vulnerable to sql injection. Assume the user of your application inserts Robert'; DROP TABLE StudentInfo;-- that would result in a query text of "insert into StudentInfo values ('Robert','',''); DROP TABLE StudentInfo;-- ', 'theName', 'theComboboxValue') " resulting in deleting your database table StudentInfo which isn't what you want !

The solution to this is using SqlParameter's which will escape the provided values in a way that this can't happen like so

string query = "insert into StudentInfo values (@first, @second, @third)";
SqlCommand command = new SqlCommand(query, cn);
command.Parameters.AddWithValue("@first", textBox1.Text.Trim());
command.Parameters.AddWithValue("@second", Name.Trim());
command.Parameters.AddWithValue("@third", comboBox1.SelectedItem.ToString());
command.ExecuteNonQuery();

I have named the parameters first, second and third, because I don't know what their meaning is, so you should give them proper names yourself. The @ needs to be at the beginning so that the names can be parsed to the desired parameters.

Naming is very important in programming, so you should always use proper names for anything you do. If you come back in 2 weeks to this piece of code you won't know for what e.g textBox1 or comboBox1 are standing.


Check non-continuous range for equal values

Using a Dictionary to count the occurences of a single value is overkill.  If the question was count the number of values that match the first then If FirstValue = cl.Value Then Count = Count + 1 would be far more efficient than using a Dictionary.

  I would like to know if all cells are equal to the first Cell in the Range

It seems to me that you are over complicating things.  Based on the objective above we are looking for a simple True or False answer.  Currently, you have to iterate over all the cells before you can determine whether all the cell value match. So it takes the same amount of time whether all cells match or not.  It would be far better to set a flag and exit the loop when a unmatched value is found.

'Get range of cells
Dim rng As Range
Set rng = Range("A1:C1,F1,K1,X1,AA1")

Dim Value As Variant
Value = rng.Cells(1, 1).Value
Dim flag As Boolean

For Each cl In rng
    If cl.Value <> Value Then
        flag = True
        Exit For
    End If
Next cl

'Test for same values
If Not flag Then
    Debug.Print "All equal to first cell"
Else
    Debug.Print "Not all equal to first cell"
End If


A number of points:

Do not use floating-point data types (float or double) for amounts of money. Floating-point data types have limited precision, they cannot represent all decimal values exactly, and you're going to get roundoff errors sooner or later. Use integers instead (int or long) and store cents instead (for example, 1040 cents) or use BigDecimal.
Make member variables private by default; only make them protected (or another access level) if there is a good reason to do so.
Prefer local variables above member variables. Is there a good reason why s is a member variable instead of a local variable?
Don't write the main part of a program in the constructor of a class. You have the main part of your program in the constructor of the Bank class. The constructor is meant to initialize the state of a new object, so the only thing that the constructor should do is initialize member variables.
In your constructor, you catch an InputMismatchException and print an error message ("bye bye...") but you are not actually returning from the constructor. So execution will continue and print "Incorrect PIN entered ..." which is not what you meant.
The program doesn't actually do what you'd expect it to do; withdrawal, deposit etc. functionality never gets called anywhere.
You need a public static void main(String[] args) method to be able to run this as a stand-alone program.


No JavaScript guru, but I do have some thoughts on this.

As you yourself have point out in comments, this will be a problem when loadJS is called with the same URL a 2nd time before the 1st call has completed:

function loadJS(absoluteUrl, onComplete){
    if(loadJS[absoluteUrl])
        return;//Script already loaded

    loadJS[absoluteUrl] = true;//Memoize
    // ...

As loadJS[absoluteUrl] gets set to true immediately,
even before the URL is really loaded,
subsequent calls to the method will return early.
By moving the loadJS[absoluteUrl] = true; line further down in the method,
somewhere after the onComplete() call,
the problem will be mitigated.
In that case,
the worst thing that can happen is that the URL will be loaded twice,
which is not as bad as the rest of the code happily executing thinking that initialization has successfully completed when in fact it hasn't yet.

Update: ok maybe that's not the worst thing that can happen (as you pointed out in comments).
For example, if the same JS file is loaded by multiple loadJS calls in parallel,
the functions in the JS file will get overwritten.
I don't know how that works, and what happens if the function is being overwritten while it's also being executed at the same time.
To avoid such issues,
you would need to implement locking in loadJS to prevent subsequent calls while a URL is being loaded.

It seems a bad idea to use the same name for objects and functions,
for loadCSS and loadJS, for many reasons:

Can be confusing and lead to nasty bugs (see more on that in my update)
loadCSS and loadJS are not good names for objects that act essentially as a cache of url -> flag, indicating whether a URL was already loaded or not
Since CSS and JS URLs are distinct, a single cache will be enough

So how about renaming the cache, and sharing between CSS and JS:

function loadCSS(absoluteUrl) {
    if (alreadyLoadedUrls[absoluteUrl])
        return;  // Css already loaded

    // ...
}

function loadJS(absoluteUrl, onComplete) {
    if (alreadyLoadedUrls[absoluteUrl])
        return;  // Script already loaded

    // ...
}

Another thing,
loadJS returns different things when the URL was already loaded or not:

If already loaded, returns null
If called for the first time, it returns onComplete()

I think you should make the behavior consistent: either return null always, or onComplete() always.

Update

As you asked, a bit more info why not to use the same name for different objects.

As a general principle, programs should be as clear as possible.
Using the same name for different objects is really just asking for trouble,
for no apparent benefit, at least in this example.
The handling of and behavior of this feature in JavaScript can be confusing,
and may also be browser-dependent.

Take a look at the example snippets in this other answer.
Predicting the output of these examples is far from trivial:

(function (foo) {
  var foo; // doesn't affect the existing `foo` identifier
  return typeof foo;
  function foo () {}
})('string'); // yields "function"

(function (foo) {
  var foo = {};
  return typeof foo;
  function foo () {}
})('string'); // yields "object"!!

Having such elements in the code that force me to think hard would be a really unnecessary obstacle.

Even though in your code there is no such confusion,
that might creep in later as you (or somebody else) add more features later.
It also sets a bad example that others might follow.
Unless there is a really good reason to do this,
and real advantages,
don't call different things by the same name,
don't make me think,
keep things perfectly clear.


on using the strategy pattern to switch between database layers

Typically you'd have two classes that implement the four methods create, read, update and delete, one for Mongo and one for PostGres. Create a factory method that determines what kind of DB you are talking to, and returns an instance of the appropriate class.
The rest of your code can interact with the class using the methods. It doesn't need to know what kind of DB you are using.


First of all, my apologies. I completely misread your code.

Ultimately the goal is to reverse bit pairs. The task is very similar to reversing the bits (this discussion is really inspiring), and the approach should be along the same lines.

For example, the first solution can be adopted along the lines of (sorry for sloppy coding):

unsigned int radix4_reverse(unsigned int index)
{
    x = (((x & 0xcccccccc) >> 2) | ((x & 0x33333333) << 2));
    x = (((x & 0xf0f0f0f0) >> 4) | ((x & 0x0f0f0f0f) << 4));
    x = (((x & 0xff00ff00) >> 8) | ((x & 0x00ff00ff) << 8));
    x = (x >> 16) | (x << 16));
    return x >> (32 - bits);
}

(notice that the 0xaaaaaaaa/0x55555555 line is omitted for it will swap bits within a pair).

Regarding your code,

ifs inside the loop are serious performance killers
1 << k is preferable to pow_2[k]. The compiler has better chances to optimize sequential shifts than sequential memory fetches. Besides, it relaxes the memory consumption.
You should operate on bit pairs (e.g. i & (0x03 << k) rather than individual bits.
| is cleaner than + (IMnsHO).
I recommend to pass bits instead of N.


Don't invent your own language through macros

GLOBAL and LOCAL might look cute but they make the code harder to read for everyone else. Not to mention the name-space pollution caused by such macros.

You say that they both expand to static. That seems wrong. A function declared static is not “very global” – it is private to its translation unit. A local variable declared static is not “very local” – it is shared between all invocations of the function. This also seems wrong. If you want to use format as a library function, it shouldn't be static. And declaring the integer initialSize as static seems pointless at best.

#include all required headers

Probably you have just left it out of your question but your code should always #include all required header files.  In your case, that would be

<cstdio> for std::vsnprintf,
<string> for std::string,
<cstddef> for std::size_t and
<cstdarg> for va_start, va_end and std::va_list.

Since you're using string without the std:: qualifier, I assume you also have a using namespace std; somewhere in your code. Get rid of it and type the extra five characters instead. It makes it clearer where the types are coming from and doesn't pollute the global name-space.

Use namespaces to organize your functions

Maybe you just didn't show it but you should put everything except main in some namespace. Instead of declaring stuff static, consider using an anonymous namespace. But since your function ought to be useful in multiple translation units, it probably shouldn't be static anyway.

Properly allocate and deallocate dynamic memory

The line

char bufferCorrectSize[length];

is not correct. (Your compiler should have warned you about it.) In C++, an array must have a fixed size known at compile-time. Since the value of length is only obtained at run-time from a previous function call, this is not possible. You could dynamically allocate a buffer of the wanted size using

char * bufferCorrectSize = new char[length];

and release it again once you're done with it.

delete[] bufferCorrectSize;

But this leaves you with all kinds of memory management issues. So instead of managing it yourself, use a std::vector<char>.

Unfortunately, you cannot use the std::string directly because its data member function will only give you a const buffer. If you know exactly what you're doing, you can take the address of the first character of the string and use it a s a pointer to a writable buffer. But I'd rather not play such tricks unless you're comfortable with what you're doing and the additional performance is critical.

Don't repeat yourself

You're calling std::vsnprintf twice; basically with the same arguments. Also, there is no need to call it again if the buffer was large enough the first time. You can re-factor this into a loop.

Get rid of the useless nested scope

While a nested scope { … } can be useful from time to time, in your case, it serves no purpose. You're not even declaring any variables inside it.

Handle errors properly

If std::vsnprintf encounters an error, it will return a negative number. You should test for this and properly handle the error. throwing an exception is probably an adequate reaction. Don't forget to still call va_end in this case.

Handle embedded NUL bytes correctly

You're returning bufferCorrectSize which will decay to char *. Unlike others have commented, this is not returning a pointer to a local object. Since format is declared with a return type of std::string and the latter is implicitly constructible from a const char *, you're safely returning the std::string as intended. However, if there is a NUL byte somewhere in the buffer, you'll truncate. Therefore, you should use the constructor taking a pointer and a length and call it explicitly.

Eliminate unused variables

The variable returnVal is never used. Your compiler might be able to warn you about this.

Make debugging easier with function attributes

The C-style printf family of functions has a bad reputation of being notoriously not type-safe and responsible for numerous bugs that can lead to serious security holes. A good compiler like GCC can warn you if such a function is called with arguments that do not match the type specifiers in the format string. You can enable the same warnings for your function, too. Of course, GCC can only check the arguments if it can see the format string. Using an untrusted format string is a very bad idea anyway so you should consider making it a const char *. If somebody really wants to call your function with a computed format string, they can always obtain a C-style string via the c_str member function of std::string. This will give them the compiler warning they deserve so they can think twice about their decision. Having format accept a std::string as format string also leads to the false assumption that one can embed NUL characters in it. But since you're going to call c_str on it anyway, std::vsnprintf will only ever see a NUL terminated string. Making the type of the format string const char * avoids this confusion.

The following annotation informs GCC that the function accepts a printf-style format string as parameter number 1 and parameters 2 and following are the format arguments.

std::string
format(const char *const format, ...)
  __attribute__ ((format (printf, 1, 2)));

See the GCC manual for more information about the (arguably messy) syntax.

Document your interface

Add a comment to the declaration of each public function that describes precisely how to use it.

Putting it all together

Combining all of the above advice, we end up with the following two files.

my_format.hxx:

#ifndef MY_FORMAT_HXX
#define MY_FORMAT_HXX

#include <string>  // std::string

namespace my
{

  /**
   * @brief
   *         Like `std::sprintf` except that the formatted string is `return`ed
   *         as a `std::string`.
   *
   * This function internally forwards to `std::vsnprintf`.  It accepts the
   * exact same parameters and has the exact same gotchas as that function
   * except that allocating a buffer of sufficient size is done automatically.
   *
   * @param format
   *         C-style format string
   *
   * @param ...
   *         any format arguments required by the given format string
   *
   * @returns
   *         the formatted string
   *
   * @throws std::runtime_error
   *         if `std::vsnprintf` fails
   *
   * @throws std::bad_alloc
   *         if insufficient memory is available
   *
   */
  std::string
  format(const char *const format, ...)
    __attribute__ ((format (printf, 1, 2)));

}  // namespace my

#endif  // #ifndef MY_FORMAT_HXX

my_format.cxx:

#include "my_format.hxx"

#include <cstdarg>    // va_start, va_end, std::va_list
#include <cstddef>    // std::size_t
#include <stdexcept>  // std::runtime_error
#include <vector>     // std::vector

namespace my
{

  std::string
  format(const char *const format, ...)
  {
    auto temp = std::vector<char> {};
    auto length = std::size_t {63};
    std::va_list args;
    while (temp.size() <= length)
      {
        temp.resize(length + 1);
        va_start(args, format);
        const auto status = std::vsnprintf(temp.data(), temp.size(), format, args);
        va_end(args);
        if (status < 0)
          throw std::runtime_error {"string formatting error"};
        length = static_cast<std::size_t>(status);
      }
    return std::string {temp.data(), length};
  }

}  // namespace my


Is hard to evaluate without the html  but, considere introduce toggleClass.

$element.toggleClass(className, condition);

http://api.jquery.com/toggleClass/

If your jQuery version doesn´t allow it, you can introduce as a separate function:

function toggleClass($element, class, condition) {
   condition ? $element.addClass(class) : $element.removeClass(class);
}

and the full solution: http://jsfiddle.net/cbhvteon/1/


Toggling a taxi between busy and available states

I voted to reopen, though I still don't consider this a good CodeReview question. Perhaps my answer here will shed some light on why I think that.

You say you want to shorten the switch_status method. Well, Ruby has ternaries, same as many other languages so:

def switch_status
  @status = (@status == :available ? :busy : :available)
end

And done. Shorter.

But the reason this question feels incomplete is because that, in and of itself, is completely equivalent to the original code. So it's not really a review, just a demonstration of an alternative if..else syntax found in many languages.

A review would require knowing more about why this method exists. What its purpose is, when you need it, why it's using symbols, and so on.

To start, just having the switch_state method is odd. You presumably have to know what state the taxi (as the class name suggests it is) is already in, in order to see if switching it makes sense. But your code contains no mechanism for determining the current state. In fact, the only way to get a Taxi instance to return something is to change its state. Calling switch_state will return the new state, and you'll have to work backwards from there or switch it back again.

E.g. suppose you have an instance, and want to know its state. Currently, you'd have to do this:

some_taxi.switch_state # flip it once
current_state = some_taxi.switch_state # flip it back again

That makes very little sense.

Also: You're switching between exactly two states - which is what booleans do already. So why aren't you using booleans? They're limited to two states, so you're certain it's either one or the other (whereas a symbol can be anything). So you could just as well do this:

def switch_status
  @busy = !@busy
end

Really, that should do the same trick, since this taxi has only two states.

And also, given the lack of context, I'd say you're entire class could, in effect, be replaced with just a boolean, since the class's only purpose right now is to store a boolean state. So cut out the middleman. A raw boolean value would even tell you its current state, which is more than your class does now.

But presumably you want the encapsulation and the symbols for something. Or do you want symbols? A more Ruby'esque way to inquire about a known set of states would be with ?-methods:

def busy?
  @busy
end

def available?
  !@busy
end

Or you can just have one of 'em and negate their value, e.g. !taxi.busy?. Either way, it's infinitely better than flipping the state twice just to find out what it currently is.

If you still want symbols, write an accessor:

def status
  @busy ? :busy : :available
end

Then again, if this is a taxi, does it really have only those two states? Maybe one is :out_of_service for instance?

Again, no idea. The question doesn't say.

Or it could be that available/busy can be - and likely should be - inferred from something else. There's no reason to manually track multiple separate state values if they're dependent on each other. In fact, it's likely harmful to try. So perhaps your code could do this instead:

def status
  @passengers.any? ? :busy : :available
end

Now status is derived from something (presumably) more meaningful.

But does this make sense? No idea. The question doesn't say.

As mentioned, just having switch_state is odd because presumably there's more to it than that: The state is connected to something. Otherwise, as I also mentioned, you could replace the whole class with a simple boolean value. Only being able to manipulate the state by flipping it seems weird. Like a light switch you can only even flip to the opposite of whatever it is - and you don't know whether that's on or off.


Looks at order data and arranges it to focus on order locations

A quick fix would be to change:

For Each cell In rng.Cells
    For Each header In rng2.Cells
        If cell.Value = 0 Then

        ElseIf cell.Value <> header.Value And s2.Cells(cell.row, header.Column).Value <> 1 Then
            s2.Cells(cell.row, header.Column).Value = 0
        Else
            s2.Cells(cell.row, header.Column).Value = 1 ' indicates that this order features a line from this location
        End If
    Next header
Next cell

to

For Each cell In rng.Cells
  If cell.Value <> 0 Then
    For Each header In rng2.Cells
        If cell.Value <> header.Value And s2.Cells(cell.row, header.Column).Value <> 1 Then
            s2.Cells(cell.row, header.Column).Value = 0
        Else
            s2.Cells(cell.row, header.Column).Value = 1 ' indicates that this order features a line from this location
        End If
    Next header
  End If
Next cell

That way you only loop through the headers if the cell value isn't 0, instead of looking at every header for every cell no matter its value, then checking to see if it's a cell you're interested in.

It also gets rid of the awkward If x do nothing else do something construct.


Something along the lines of this. Generics aren't that difficult. Just replace every occurance of 'some desired type' with the generic identifier.

private void AddUpdateRow<T>(BindingSource bindingSource, List<T> cachedList)
    where T : ITransmitDocId
{
    try
    {
        bindingSource.EndEdit();
        T t = bindingSource.Current as T;
        if (t.TRANSMIT_DOC_ID == 0)
        {
            if (!cachedList.Contains(t))
            {
                cachedList.Add(t);
            }
            return;
        }
        context.UpdateObject(t);
    }
    catch (Exception ex)
    {
        logger.ErrorException(string.Empty, ex);
    }
}

The where constraint in the function definition should indicate a common interface/class in which TRANSMIT_DOC_ID is defined.


A simple Java game where the player guesses the word

Performance

You create a new Scanner for every guess. this is unnecessary. the same scanner can be used to read all user input. the scanner should be closed only at the end of the program. and if you adopt that, you can also use Java 7 try-with-resources feature. it is a safe(r) way to open and process external resources.

JDK library features

in order to fill an array with the same character, you can use Arrays.fill() method: Arrays.fill(censoredWordArray, '_'); and you do not need to keep the String representation of the array, just print the string value to the user. However, FYI, Java 11 gives us String::repeat to fill a String with one character: String censoredWord = "_".repeat(word.length());
I assume this is CS course homework, so I do not know if this comment is applicable. However, Java 8 (which has been around for over 5 years) gave us stream processing of collections, which, compared to for loops, is a more expressive (and sometimes performant) way to iterate over collections and arrays. for example, the loop in showProgress can be re-written like this

IntStream.range(0, wordArray.length)
    .filter(i -> wordArray[i] == letter)
    .findFirst()
    .ifPresent(i -> censoredWordArray[i] = letter);

Best practices

Avoid magic numbers and literals:
private static final String EMPTY_LETTER = "_";
not only it makes the code more readable, it will eliminate typo bugs in case you need to specify the literal in multiple places in the code.


Incremental Sieve of Eratosthenes using iterators

Your code is totally fine and idiomatic, but I would change some minor points:

use std::collections::HashMap;

struct Sieve {
    composites: HashMap<u64, Vec<u64>>,
    current: u64,
}

impl Default for Sieve {
    fn default() -> Self {
        Sieve {
            composites: HashMap::new(),
            current: 2,
        }
    }
}

impl Sieve {
    pub fn new() -> Sieve {
        Default::default()
    }
}

impl Iterator for Sieve {
    type Item = u64;

    fn next(&mut self) -> Option<u64> {
        fn next_prime(composites: &mut HashMap<u64, Vec<u64>>, x: u64) -> u64 {
            match composites.get(&x) {
                Some(numbers) => {
                    for num in numbers.to_owned() {
                        composites
                            .entry(x + num)
                            .and_modify(|v| v.push(num))
                            .or_insert_with(|| vec![num]);
                    }
                    composites.remove(&x);
                    next_prime(composites, x + 1)
                }
                None => {
                    composites.insert(x * x, vec![x]);
                    x
                }
            }
        }

        let prime = next_prime(&mut self.composites, self.current);
        self.current = prime + 1; // This number will be the next to be tested

        Some(prime)
    }
}

fn main() {
    let mut sieve = Sieve::new();

    assert_eq!(sieve.next(), Some(2));
    assert_eq!(sieve.next(), Some(3));
    assert_eq!(sieve.next(), Some(5));
    assert_eq!(sieve.next(), Some(7));
}

It is good to implement Default when you can build your data structure without parameters (related answer).
This is a good practice to put assertions instead of prints. That's easier to change to code and verify that it is still ok.
The naming of the variables is important; but I do not pretend that mine is perfect, though. By the way, _num should not be prefixed with an underscore because it is used.
You can run clippy to catch some common error. Clippy warns you that you create a vec![_num] at each iteration. You should give to or_insert_with a closure that will build the correct vector.
The more important refactoring that I did is to make the algorithm more "functional". I think that yours is difficult to reason about because it calls recursively the Iterator::next method and rely on internal state.


Should jQuery DOM Ready code be in the page with the HTML or separate JavaScript file?

I don't know if it is good practice, but in my eyes it would be cleaner, since you have several JS-files at this point, to separate markup, layout and JS completely. So $() would reside in a .js-File.

In some cases, when you only have a few lines of JS or -on the other hand- if you have to care for every millisecond to deliver a good page experience, it would make sense to embed JS in the html-file and retrieve the non-critical JS at later time dynamically.

It depends on your usecase.


This code looks pretty good, but you seem to have largely reinvented parse_ini_file($filename, false, INI_SCANNER_TYPED).  All you need to add is the support for the search path.

do_parse() is inconsistent about returning a status code, and parse_file() fails to check it.

You should call explode("=", $line, 2), so that any = character in the value will be interpreted literally.


Sh script to reduce version number in opkg status file

Since you're already using awk in there,
it would be better to rewrite all the echo - grep - sed processing with a single awk.
In particular, the entire while loop in the middle can be replaced with this:

awk -F '[:. ]' '/^Version: / {
  num1 = $3;
  num2 = $4;
  if (num1 > 0) {
    gsub(num1, num1 - 1);
    print;
  } else if (num2 > 0) {
    gsub(num2, num2 - 1);
    print;
  }
}
' "${BACKUP_OPKG_STATUS_FILE}" >> "${OPKG_STATUS_FILE}"

This produces slightly different (I think better) output than your original,
and with the number of sub-processes greatly reduced,
it's a lot more efficient.


Definitely readable although I would just combine your two select statements and add some whitespace:

if (_queue.OrderByDescending(record => record.Primary)
          .GroupBy(record => record.Term)
          .Select(record => Create(record.First()))
          .Any(result => result <= 0))
{
    throw new ModelException("Failed to create keyword record.");
}

As you see I also prefer to keep the first statement on the same line and indent the next statements to align with the linq statement.

One remark though: first you're ordering your records and then grouping them. Is this done deliberately? I would assume that you'd want to group first and then order, not in the least because you would have to order less elements. But then again: grouping might be more performant if you order it first so I'm not entirely sure.


CSV to Object converter with JavaDoc comments

Prefer interfaces to implementations

private ArrayList<TestData> loadTestData(String calendar) {
    ArrayList<TestData> testDataArray = new ArrayList<>();

and

        ArrayList<String> fields = CSVReader.parseLine(lines[i]);

and

        ArrayList<String> correctHostCodes = CSVReader.parseLine(fields.get(3));
        ArrayList<String> correctParticipantCodes = CSVReader.parseLine(fields.get(4));
        ArrayList<String> correctPhoneNumbers = CSVReader.parseLine(fields.get(5));

You would normally write these as

private List<TestData> loadTestData(String calendar) {
    List<TestData> testDataArray = new ArrayList<>();

and

        List<String> fields = CSVReader.parseLine(lines[i]);

and

        List<String> correctHostCodes = CSVReader.parseLine(fields.get(3));
        List<String> correctParticipantCodes = CSVReader.parseLine(fields.get(4));
        List<String> correctPhoneNumbers = CSVReader.parseLine(fields.get(5));

That way, even if parseLine changes its return type, you don't have to change unless it changes to something that doesn't implement the List interface.

Range-based for loops

    String[] lines = calendar.split("\r\n");

    // always skip lines[0] as it contains the headers
    for (int i = 1; i < lines.length; ++i) {
        ArrayList<String> fields = CSVReader.parseLine(lines[i]);

You could rewrite this as

    List<String> lines = Arrays.asList(calendar.split("\r\n"));

    // always skip lines[0] as it contains the headers
    for (String line : lines.sublist(1, lines.size())) {
        List<String> fields = CSVReader.parseLine(line);

Or not.  If you find the indexed form easier to follow, that works too.


For some kind of fun I've replaced your byte array with the buildin BitArray (as mentioned by Dmitry) in you class just to see the effect (it seems to be slightly faster). Further I've tried to implement the sieve algorithm as linq based. It all goes like this:

    using System;
    using System.Collections;
    using System.Collections.Generic;
    using System.Diagnostics;
    using System.Linq;

    namespace PrimesAndSieve
    {
      public class SieveOfEratosthenes
      {
        private readonly int highestNumber;

        private readonly BitArray data;

        private SieveOfEratosthenes(int highestNumber)
        {
          if (highestNumber < 1)
          {
            throw new ArgumentOutOfRangeException(nameof(highestNumber));
          }
          this.highestNumber = highestNumber;
          data = new BitArray(highestNumber);
        }

        public static SieveOfEratosthenes CreateSeive(int highestNumber)
        {
          var result = new SieveOfEratosthenes(highestNumber);
          result.Populate();
          return result;
        }

        public bool IsPrime(int number)
        {
          if (number < 0 || number > highestNumber)
          {
            throw new ArgumentOutOfRangeException(nameof(number));
          }
          // 0 not stored in byte array so we have to test separately.
          //if (number == 0) ... as noted by Heslacher
          //{
          //  return false;
          //}
          return !data[number];
        }

        private void Populate()
        {
          var sqrt = Math.Sqrt(highestNumber);

          for (var i = 2; i < sqrt; i++)
          {
            if (data[i]) // not a prime
            {
              continue;
            }
            // j += i can overflow so need to guard against that in for loop.
            for (var j = i * i; j < highestNumber && j > 0; j += i)
            {
              data[j] = true;
            }
          }
          data[0] = true; // Mark 0 as not prime.
          data[1] = true; // Mark 1 as not prime.
        }
      }

  class PrimesBySieveAndLinq
  {
    public IEnumerable<long> GetPrimes(long max)
    {
      yield return 2;

      BitArray bits = new BitArray((int)max);

      foreach (var n in EnumOddNumbers(max).Skip(1))
      {
        if (!bits[(int)n])
        {
          yield return n;
          for (long j = n * n; j < max && j > 0; j += n)
          {
            bits[(int)j] = true;
          }
        }
      }
    }

    IEnumerable<long> EnumOddNumbers(long max)
    {
      for (long i = 1; i < max; i += 2)
      {
        yield return i;
      }
    }
  }

      class Program
      {
        static void Main(string[] args)
        {
          int max = 10000; // 000000; // int.MaxValue;
          var sw = Stopwatch.StartNew();
          var sieve = SieveOfEratosthenes.CreateSeive(max);

          for (int i = 1; i < max; i++)
          {
            if (sieve.IsPrime(i))
            {
              Console.WriteLine(i);
            }
          }
          sw.Stop();
          var firstDuration = sw.ElapsedMilliseconds;

          Console.ReadLine();

          sw = Stopwatch.StartNew();
          PrimesBySieveAndLinq pbsl = new PrimesBySieveAndLinq();
          foreach (var prime in pbsl.GetPrimes(max))
          {
            Console.WriteLine(prime);
          }
          sw.Stop();
          Console.WriteLine();
          Console.WriteLine("Took {0}ms", firstDuration);
          Console.WriteLine($"Duration: {sw.ElapsedMilliseconds}");
          Console.WriteLine("END");
          Console.ReadLine();
        }
      }
    }

Note: If you try PrimesBySieveAndLinq.GetPrimes(int.MaxValue) it is very slow at the beginning, but speeds up after some 1000 of primes.


No need for split (you can use readlines). But no need for readlines, for that matter:

def read_integers(filename):
    with open(filename) as f:
        return map(int, f)

or, if that’s more to your fancy:

def read_integers(filename):
    with open(filename) as f:
        return [int(x) for x in f]

A file object is simply iterable in Python, and iterates over its lines.

Note that, contrary to what I said earlier, wrapping the file object in a with block is highly recommended. Python doesn’t actually guarantee (although it recommends it) that objects are disposed of automatically at the end of the scope. Worse, it’s not guaranteed that a file object that is collected actually closes the underlying stream.

(Note that I’ve adapted the method name to Python conventions.)


It depends on what you're looking to be safe.
If you're worried about the bobby tables case, you're fine. However, there are many risks that come with user input cases.
For such a simple script, little can go wrong. However, below is only one concern that can happen (however it is probably you're most threatening):
Cross-site Scripting (XSS)

Cross-Site Scripting (XSS) attacks are a type of injection, in which
malicious scripts are injected into otherwise benign and trusted web
sites. XSS attacks occur when an attacker uses a web application to
send malicious code, generally in the form of a browser side script,
to a different end user. The end user’s browser has no way to know
that the script should not be trusted, and will execute the script.
Because it thinks the script came from a trusted source, the malicious
script can access any cookies, session tokens, or other sensitive
information retained by the browser and used with that site. These
scripts can even rewrite the content of the HTML page. (OWASP)

Basically, without validating what comes out of your data source, someone could input something nasty, and therefore giving anyone else who sees that data something nasty.
So to best protect yourself, sanitize any output from the data source and know the various attacks. Here's a good read on SO.

Also, it'd be great if you could format your code so it's more consistent and readable. Such as giving appropriate spacing around operators, correct indentations, and simply better variable names (why shorten $dbname, why CAPS $DATA?).


Improving time complexity for Count the Islands DFS

Given there are only 22-28 tiles in a Hive game, the time complexity of the check probably doesn't matter much.  Nevertheless, a modified scanline fill algorithm might be faster than a DFS.  This answer explains the algorithm fairly well. In this use case it might be easier to scan down the columns instead of across the rows.
From the rules of the Hive game, it seems the "one hive rule" would need to be checked when a tile is put down on the playing field and when it is picked up from the playing field (e.g., to move it).  When a tile is put down, it is sufficient to make sure it touches another tile (beside or on top).
When a tile is picked up, it is necessary to make sure it's former neighbor tiles are still connected via another path.  This may be trivial such as when the neighbors are adjacent, or may require searching for a path using a DFS, BFS, spanning tree, or fill/paint algorithm.
In any case, the AI should know which tile is being moved, so it shouldn't be necessary to scan the hex grid.  Just start from one of the tiles neighbors.


How can I use vector for entering numbers?

Formatting

This is probably the worst formatted piece of code I have seen in a while. It does not matter to the compiler but you are writing in a high level language so that other humans can read it.

Indentation
White Space
one line per declaration
Use meaningful variable and function names (don't shorten them).

Examples:

void student::vrod(){
cout<<"name=";
cin>>name;
cout<<"stno=";
cin>>stno;
cout<<"score=";
cin>>score;
}

// this should be (more like)

void student::vrod()
{
    std::cout << "name=";
    std::cin >> name;

    std::cout << "stno=";
    std::cin >> stno;

    std::cout << "score=";
    std::cin >> score;
}

Declarations. The types of objets are the most important thing in C++. So give each object a name and a type and use a single line for each.

    int stno,score,i,n;

    // That's just lazy;

    int stno;   // Why not studentNumber
    int score;
    int i;
    int n;

Stop Using "using"

using namespace std;

See: Why is “using namespace std” in C++ considered bad practice?

For anything longer than a 10 line program it becomes a curse and is likely to get you into trouble (in a way that is hard to find). So avoid doing it. Also avoid using it in your 10 line programs as that can be habit forming and will make you use it accidently).

Nameing conventions.

It is common convention for user defined types to have an initial capitol letter. This is to make types stand out from objects.

MyThing();  // This is a temporary object that is created.
myThink();  // This is a function call
            //
            // Notice the initial capitol letter allows you
            // to distinguish the two.

Also member variables should be nouns (the names of things, descriptive). While methods should be verbs (actions that are applied to your object). Using longer names will not make the code larger but it will make the code more readable.

class Student
{
    public:
        // Method names are actions that can be applied to
        // the object (avoid names like get/set it breaks encapsulation

        void getInfo(std::istream& str = std::cin);
        void display(std::ostream& str = std::cout);
    private:
        // Member variables should always be private.
        // Any modification of the state of the object
        // should be controlled by a method of the class
        // so that state can be updated consistently.

        // Use meaningful names.
        // Note: These are inside the student class.
        //       So name  => is a student name
        //          score => is a student score

        std::string   name;   // You seem to have forgotten this member.
        std::size_t   number;
        int           score;
        // These seem redundant so I am removing them.
        // int           i;
        // int           n;
};

Input and Output operator.

Your functions. vrod() and dis() work fine. But they only accept input/ouput from the standard streams. It would be nice if they could accept input/output from any stream (like a string or a file or a network socket).

You should pass the stream they are going to use as a parameter. You can default it to the stanadrd stream so that if it is not specified then it uses std::cin/std::cout.

class student{
public:
    void vrod(std::istream& in = std::cin);
    void dis(std::ostream& out = std::cout);
};

Then normal input and output is done via operator<< and operator>>. Writting these for your class becomes very simple.

std::ostream& operator<<(std::ostream& s, student const& data)
{
    data.dis(s);
    return s;
}
std::istream& operator>>(std::istream& s, student& data)
{
    data.vrod(s);
    return s;
}

Now you can print your object just like other objects.

std::cin << st[2];

Overall

The first prgram is better than the second program because of the encapsulation of student. BUT arrays are not very flexable and you should be using std::vector<student> st; ratheer than student st[size];. The ability to resize, lazily construct objects, always know the its size are three definite advantages that vector has over an array.


I first propose how to improve your algorithm without changing the data structures.

Your algorithm is quadratic (two nested loops over all the entries). This can be easily avoided because you don't need to loop over all the g array to find the item you need.

If they are not, sort the arrays by the id key, using sort_by:

r.sort_by! { |e| e[:id] }
g.sort_by! { |e| e[:id] }

Then you can use each_with_index:

r.each_with_index { |result, i|
    guess = g[i]

    # compute the score here
}

or, if speed is not an issue (and this does not seem to be the case), you can also zip the arrays together r.zip(g) and iterate on that:

r.zip(g).each { |result, guess|
    # compute the score here
}

If you can modify the data structure I propose some alternatives.

Often when you have a set of objects with an id you use a hash.

# g = guesses
g = {1 => { elmer: 5, roger: 7, outcome: "Roger Win" }, 2 => {elmer: 5, roger: 1, outcome: "Elmer Win" }, 3 => { elmer: 4, roger: 8, outcome: "Roger Win" }}
# r = actual results
r = {1 => { elmer: 3, roger: 9, outcome: "Roger Win" }, 2 => {elmer: 7, roger: 9, outcome: "Roger Win" }, 3 => { elmer: 4, roger: 8, outcome: "Roger Win" }}
# points table
p = {}

# Loop over results.
r.each { |id, result|
  guess = g[id]

  # Init a hash to store score
  score = {}

  # compute the score here
  # ...

  p[id] = score unless score.empty?
}

If the id is a simple incremental integer and you add the entries in r and g in the same order, you can avoid the id at all and use the array indices instead:

# g = guesses
g = [{ elmer: 5, roger: 7, outcome: "Roger Win" }, {elmer: 5, roger: 1, outcome: "Elmer Win" }, { elmer: 4, roger: 8, outcome: "Roger Win" }]
# r = actual results
r = [{ elmer: 3, roger: 9, outcome: "Roger Win" }, {elmer: 7, roger: 9, outcome: "Roger Win" }, { elmer: 4, roger: 8, outcome: "Roger Win" }]
# points table
p = {}

# Loop over results.
r.each_with_index { |result, id|
  guess = g[id]

  # Init a hash to store score
  score = {}

  # compute the score here
  # ...

  p[id] = score unless score.empty?
}

You can create a class to represent each entry of the array (let's call each entry a Match) and then implement the scoring algorithm as a method:

class Match
  attr_accessor :elmer, :roger, :outcome

  def initialize(elmer, roger, outcome)
    @elmer = elmer
    @roger = roger
    @outcome = outcome
  end

  def compare(m)
    score = 0

    score = score + 1 if @outcome == m.outcome
    score = score + 4 if @elmer == m.elmer and @roger == m.roger

    score
  end
end

Example usage:

guess = Match.new(5, 7, "Roger Win")
result = Match.new(3, 9, "Roger Win")
puts guess.compare(result) # => 1

guess = Match.new(5, 1, "Elmer Win")
result = Match.new(7, 9, "Roger Win")
puts guess.compare(result) # => 0

guess = Match.new(5, 7, "Roger Win")
result = Match.new(5, 7, "Roger Win")
puts guess.compare(result) # => 5

puts guess.compare(guess) # => 5

If the string argument just says who is the winner based on the numbers, you can compute it:

class Match
  attr_accessor :elmer, :roger

  def initialize(elmer, roger)
    @elmer = elmer
    @roger = roger
  end

  def outcome
    if @elmer > @roger
      :elmer
    elsif @roger > @elmer
      :roger
    else
      :tie
    end
  end

  def compare(m)
    if @elmer == m.elmer and @roger == m.roger
      5
    elsif outcome == m.outcome
      1
    else
      0
    end
  end
end

Example usage:

guess = Match.new(5, 7)
result = Match.new(3, 9)
puts guess.compare(result) # => 1

guess = Match.new(5, 1)
result = Match.new(7, 9)
puts guess.compare(result) # => 0

guess = Match.new(5, 7)
result = Match.new(5, 7)
puts guess.compare(result) # => 5

puts guess.compare(guess) # => 5

guess = Match.new(2, 2)
result = Match.new(3, 3)
puts guess.compare(result) # => 1

guess = Match.new(2, 2)
result = Match.new(2, 2)
puts guess.compare(result) # => 5


One tip: use the argparse module.
import argparse

parser = argparse.ArgumentParser()

parser.add_argument("--log_file", dest="log_file", type=str, required=True, help="Add some help text here")
parser.add_argument("--input_file", dest="input_file", type=str, required=True, help="Add some help text here")

args = parser.parse_args()

# show the values, will reach this point only if the two parameters were provided
print(f"log_file: {args.log_file}")
print(f"input_file: {args.input_file}")

Then you call your script like this:
python3 generate.py --log_file test.log --input_file test.txt

Parameter order is free.
I have to admit I don't understand much about the purpose since I don't know about your input files.
If they are CSV then you might consider using the csv module. Then you should be able to simplify some statements like this one:
total_force=[float(line.split()[3]) for line in log if line.rfind('Total force =') != -1][-1]

Something is lacking in your script, badly: comments. They will help you too, especially when go back to reviewing code you wrote a few months ago. Probably you will have forgotten details and will have to reanalyze your own code.


I'm not a Python programmer, but the way to not repeat the code without creating a function is to do the download checking first have flags set by that checking one for creating a new file path and one for doing the download. Figure out everything that needs to be done first and then do the download and path creation as necessary.
pseudo code:
if not local path or file:
    create_local_path_flag = True

if file size not valid:
    file_out_of_date_flag = True

if create_local_path_flag or file_out_of_date_flag:
    if create_local_path_flag:
        create_local_path()
    download_file()