Program for entering 1000 random numbers into two different databases, one for odd numbers and one for even I have created a program in java for entering 1000 random numbers into two different database one for odd numbers and one for even. The code is executing fine but it takes nearly a minute to execute. How can I minimize the execution time?

The source code:

import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.SQLException;
import java.text.DecimalFormat;
import java.text.NumberFormat;
import java.util.Random;

public class Test1 extends Thread {

public static void main(String[] args) throws ClassNotFoundException, SQLException {
long start = System.currentTimeMillis();

int evencount = 0;
int oddcount = 0;
int breakcon = 0;
int breakcon1 = 0;

Class.forName("com.mysql.jdbc.Driver");
Connection con = DriverManager.getConnection("jdbc:mysql://localhost:3306/db1" + "?useSSL=false", "root",
        "1234");
Connection conn = DriverManager.getConnection("jdbc:mysql://localhost:3306/db2" + "?useSSL=false", "root",
        "1234");

try {

    Class.forName("com.mysql.jdbc.Driver");

    for (int n = 1; n <= 1000; n++) {
        Random r = new Random();
        int val = r.nextInt(100000);

        if (val % 2 == 0) {
            PreparedStatement ps = con.prepareStatement(" insert into try values (?)");
            ps.setInt(1, val);
            ps.executeUpdate();
            ps.addBatch();
            breakcon = breakcon + 1;
            if (breakcon % 500 == 0 || breakcon == val)
                ps.executeBatch();
            evencount++;

        } else {
            try {

                Class.forName("com.mysql.jdbc.Driver");
                PreparedStatement ps3 = conn.prepareStatement(" insert into try1 values (?)");
                ps3.setInt(1, val);
                ps3.executeUpdate();
                ps3.addBatch();
                breakcon1 = breakcon1 + 1;
                if (breakcon1 % 500 == 0 || breakcon1 == val)
                    ps3.executeBatch();

                oddcount++;

            }

            catch (Exception e2) {
                System.out.println(e2);
            }
        }
    }

}

catch (Exception e) {
    System.out.println(e);
}

long end = System.currentTimeMillis();
NumberFormat formatter = new DecimalFormat("#0.00000");
System.out.println("Execution time is " + formatter.format((end - start) / 1000d) + " seconds");
System.out.println(oddcount + evencount);

}
}

Structure your code

Name your variables appropriately! It is not a good idea to have the connections called con and conn. Likewise ps and ps3 are not good names for your prepared statements. It will confuse you when trying to debug this.
Make a method that inserts a value into a database.
Do not catch Exception. Make sure you know what exception you catch and why. Right now you declare your main to throw exceptions, but I don't think it really does.

Prepared statements are reusable

You prepare a statement to let the database optimize for it. It takes a bit of time, but it saves time when you use it. What you do, is you prepare the statement inside your loop, making it a lot slower. Move the preparation of your statement

PreparedStatement ps = con.prepareStatement("insert into try values (?)");

outside your loop. This will make your program run faster.

Random is reusable

You create a new instance of Random on every rotation of the loop. Move this outside the loop - it will make your code run faster.

Remove unnecessary things

Your code is littered with statements executing expensive methods to no use.

What does

Class.forName("com.mysql.jdbc.Driver");

do?

What is the purpose of

ps.addBatch();
breakcon = breakcon + 1;
if (breakcon % 500 == 0 || breakcon == val)
  ps.executeBatch();

What is the purpose of oddcount and evencount?


Simple property formatting In a VB.Net class, I have an method that replaces all square brackets [] in certain properties by normal brackets ().

Edit:
The program allows users to write custom calculations, such as [3-(6-2)]. Being able to use two kinds of brackets makes things easier for them, but when processing the calculation, it's easier to have the same brackets everywhere.

The code is really redundant:

Public Sub removeBadBrackets()
    ValueCalc = Replace(Replace(ValueCalc, "[", "("), "]", ")")
    TargetCalc = Replace(Replace(TargetCalc, "[", "("), "]", ")")
    LLLCalc = Replace(Replace(LLLCalc, "[", "("), "]", ")")
    LLCalc = Replace(Replace(LLCalc, "[", "("), "]", ")")
    LCalc = Replace(Replace(LCalc, "[", "("), "]", ")")
    HCalc = Replace(Replace(HCalc, "[", "("), "]", ")")
    HHCalc = Replace(Replace(HHCalc, "[", "("), "]", ")")
    HHHCalc = Replace(Replace(HHHCalc, "[", "("), "]", ")")
End Sub

I think this could be done in a more efficient way, and I'd like to learn how.

Side note:

Those properties are not the only one of the object.
They are in the same region (although I am not sure regions can be used for anything else than my own convenience).

Thank you for your help improving this.

My VB.NET is incredibly rusty but you should write a separate function:

Public Shared Sub ReplaceBracketsWithParentheses(ByRef text As String)
    text = Replace(Replace(text, "[", "("), "]", ")")
End Sub

Calling code will then simply be:

Public Sub removeBadBrackets()
    ReplaceBracketsWithParentheses(ValueCalc)
    ...
End Sub

Note that if you have to replace more than few substrings then a fluid approach may be more readable:

Public Shared Sub ReplaceBracketsWithParentheses(ByRef text As String)
    text = text.Replace("[", "(") _
               .Replace("]", ")") _
               .Replace("{", "(") _
               .Replace("}", "]")
End Sub

If you're translating some sort of mathematical expressions (but not only!) you may also consider to use a Regex.

One word about naming: I don't know the domain you're using this code but to me LLLCalc, LLCalc and so on are not descriptive names (and, anyway, acronyms should be cased PascalCase like LllCalc, the same is true for removeBadBrackets()).


Improvement on data normalization I have a part of code that is loading a dataset and normalizing property values to [0, 1]. My implementation is:

import pickle
import numpy as np

# -- load data
prop_1    = list(np.random.rand(10)*20)
prop_2    = list(np.random.rand(10)*10)
prop_3    = list(np.random.rand(10)*30)

# -- normalize
l_bound = []
l_bound.append(min(prop_1))
l_bound.append(min(prop_2))
l_bound.append(min(prop_3))
u_bound = []
u_bound.append(max(prop_1))
u_bound.append(max(prop_2))
u_bound.append(max(prop_3))

prop_1 = (np.array(prop_1) - l_bound[0]) / (u_bound[0] - l_bound[0])
prop_2 = (np.array(prop_2) - l_bound[1]) / (u_bound[1] - l_bound[1])
prop_3 = (np.array(prop_3) - l_bound[2]) / (u_bound[2] - l_bound[2])

However, the normalizing part of the code does not look graceful. Any suggestions on how to improve it? Can do this using a loop?

Since you have numpy arrays, you should use their vectorized methods wherever possible. This can make your code a lot faster:

In [1]: x = np.arange(10000000)

In [2]: %timeit max(x)
988 ms ± 42.6 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)

In [3]: %timeit x.max()
9.67 ms ± 114 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

This includes not casting your arrays to list.

I would also make this a function that normalizes a single array:

import pickle
import numpy as np
from typing import Iterable, Any

def normalize_one(x: Iterable[Any]) -> np.ndarray:
    if not isinstance(x, np.ndarray):
        x = np.array(list(x))
    low, diff = x.min(), x.ptp()
    return (x - low) / diff

# -- load data
prop_1    = np.random.rand(10)*20
prop_2    = np.random.rand(10)*10
prop_3    = list(np.random.rand(10)*30

# -- normalize
prop_1 = normalize_one(prop_1)
prop_2 = normalize_one(prop_2)
prop_3 = normalize_one(prop_3)

If you do have many arrays that need to be normalized, you can always do it in a list comprehension:

properties = [prop_1, prop_2, prop_3]
properties = [normalize_one(prop) for prop in properties]

If you have many of them and they all have the same structure, I would use something like this (now limited to numpy arrays as input):

def normalize(x: np.ndarray, axis: int = 1) -> np.ndarray:
    """Normalize the array to lie between 0 and 1.
    By default, normalizes each row of the 2D array separately.
    """
    low, diff = x.min(axis=axis), x.ptp(axis=axis)
    # Indexing needed to help numpy broadcasting
    return (x - low[:,None]) / diff[:,None]

properties = np.random.rand(3, 10)
properties[0] *= 20
properties[1] *= 10
properties[2] *= 30

properties = normalize(properties)

For props = np.random.rand(10000, 10) I get the following timings:

Author      Timed function call                       Time [s]
Blade*      list(normalize_blade(props))              68.7 ms ± 749 µs
Linny       list(normalize_linny(*props))             127 ms ± 1.42 ms
Graipher    [normalize_one(prop) for prop in props]   119 ms ± 7.4 ms
Graipher    normalize(props)                          2.32 ms ± 113 µs

The code I used for the test with the code in the OP is this one, which is just the generalization to many properties:

def normalize_blade(properties):
    l_bound, u_bound = [], []
    properties = [list(prop) for prop in properties]
    for prop in properties:
        l_bound.append(min(prop))
        u_bound.append(max(prop))
    for i, prop in enumerate(properties):
        yield (np.array(prop) - l_bound[i]) / (u_bound[i] - l_bound[i])


Handling templates for a web application I am writing up a class to handle various templates to be used in a web application. Each template contains various placeholders which will need to be replaced at the time of the build. I am wondering if the method I am using is the best solution, or if I should consider something else. At the moment I have two separate functions addPlaceholder() and addMultiplePlaceholders(). Each performs as their names suggest.

The function addPlaceholder() has two parameters one, the key $k, must be a string while the other, the value $v, can be anything. These get stored in the classes placeholder array.

The function addMultiplePlaceholders() has only one parameter, $array, and it should be an associative array. This function calls addPlaceholder() for each index of $array to store the corresponding values. The reason for the call to an existing function is to help alleviate duplicitous code, and when only a single placeholder is in use to bypass the foreach loop.

Is this the best method to accomplish this? Would it be better to forgo addPlaceholder() and just use addMultiplePlaceholders() for everything? Perhaps, overloading addPlaceholder() would be a better solution having one version which accepts the two parameters, and another which accepted the array. However, I would be in the same situation as now having to decide whether to duplicate the code for adding the placeholder to the class property, or calling the other function.

public function addPlaceholder($k,$v){
    if(empty($k)){
        throw new Exception('Class Template->addPlaceholder() '.'The placeholder string cannot be empty.');
    }

    if(isset($this->placeholders[$k])){
        throw new Exception('Class Template->addPlaceholder() '.'['.$k.'] is a duplicate entry');
    }

    $this->placeholders[$k] = $v;
}

public function addMultiplePlaceholders($array){
    if(!is_associative($array)){                    // if `$array` is not an associative array throw exception
        throw new Exception("The supplied parameter value is not an associative array");
    }

    foreach($array as $key => $value){
        try{
            addPlaceholder($key,$value);
        }catch(Exception $e){
            $msg = $e->getMessage();
            preg_replace(preg_quote('Class Template->'), '', $msg);
            throw new Exception("Class Template->addMultiplePlacerholders()->".$msg);
        }
    }
}

To answer just the question in the title;
Your solution seems fine to me; addMultiplePlaceholders() is a utility method that acts as a wrapper for addPlaceholder()

Note there's a bug in your code; addMultiplePlaceholders() calls addPlaceholder() without $this-> 'prefix', so your code won't work

However, there is room for improvement. I'll try to summarize what I'd do differently.

Remove redundant information from your Exception messages

Exceptions are 'smart' messages in that they already contain all the meta-information you need to find where they occurred (e.g. line number, file etc). Don't clutter your messages with that information, It'll only complicate your code (for example the preg_replace). Just include the bare error-message. See the documentation on Exceptions.

If your going to output a message, make sure they cannot be mis-interpreted. Being consistent in your messages will prevent confusion and make it easier to debug your code later on, for example:

throw new Exception('The placeholder string cannot be empty');

Is confusing; What is the placeholder string? The name ($k)? Value ($v)?

Outputting the meta-information can be done when printing the exception, for example;

Just use this to throw your exception;

throw new Exception('Placeholder name cannot be empty');

And output it in your Exception-handler with this;

printf(
    "An Exception has occurred: '%s' in file '%s' on line '%s',
    $e->getMessage(),
    $e->getFile(),
    $e->getLine()
);

// --> An Exception has occurred: 'The placeholder string cannot be empty' in file 'Template.php' on line '52'

Since you're no longer including the name of the Class/Method in your messages, you can completely remove the 'try/catch' from your 'addMultiplePlaceholders'.

Use the right 'type' of Exception

To be more expressive/specific when throwing Exceptions, consider using special Exceptions for a situation, for example, the SPL Exceptions. In this case an InvalidArgumentException will be appropriate

Consider what really is an exception

Inside addPlaceholder, you're throwing an exception if a key is already set. Personally, I wouldn't do this (it does feel like 'business logic'). Technically, overwriting the value of a placeholder that was already set won't cause any problem. Disallowing overwriting existing placeholders may limit you in situations that you want to overwrite a placeholder.

However, renaming addPlaceholder to setPlaceholder may be more appropriate, to indicate that you can both 'add' or 'update/replace' a placeholder.

Use descriptive variable/argument-names

The arguments for addPlaceholder are not very descriptive. Additionally, the variable names you're using seem to be picked based on the the 'inner workings' of the method (key/value pairs), which should be of no interest when using that method. I'd suggest something more describing the purpose of the arguments, for example:

public function addPlaceholder($name, $value)
{
    //...
}

Likewise, replace '$array' to something more describing its purpose, for example

public function addMultiplePlaceholders($nameValuePairs)
{
    //...
}

// or..

public function addMultiplePlaceholders($placeholders)
{
    //...
}

If you want to explicitly indicate you're expecting an array (and have PHP check this for you), you can add a type-hint, like this:

public function addMultiplePlaceholders(array $placeholders)
{
    //...
}

Finally, naming this method addMultiplePlaceholders may be a bit verbose. Since the method is plural, it's probably already clear that it is used to add multiple placeholders, so simply calling it addPlaceholders may be clear enough.

Remove 'comment' clutter. Add PhpDoc in stead

Do not describe code that is self-explanatory. In general, try to omit inline comments in all cases, unless a piece of code really is confusing.

In most cases, don't describe the code, but describe the purpose / intent of a function/method inside a PhpDoc comment.

For example:

if(!is_associative($array)){ // if `$array` is not an associative array throw exception
    throw new Exception("The supplied parameter value is not an associative array");
}

This comment is just repeating the code! Also, inline code is not used by IDE's and will not be included in automatically generated documentation (e.g. phpdocumentor). It's best to move relevant information to a PhpDoc block

For example:

/**
 * Adds placeholder(s) passed as an associative array ('name' => value)
 *
 * Name should be a string, value can be any type of value
 *
 * This method is a wrapper for @see addPlaceholder()
 *
 * @param array $nameValuePairs
 *
 * @throws InvalidArgumentException if $nameValuePairs is not an associative array
 *                                  or an empty name is used for a placeholder
 * @return void
 */
public function addMultiplePlaceholders(array $nameValuePairs)
{
    //...
}

Modified code

This is the code after applying my suggestions

/**
 * Adds or updates a placeholder
 *
 * @param string $name  Name of the placeholder. Cannot be empty
 * @param mixed  $value Value of the placeholder
 *
 * @throws InvalidArgumentException if $name is invalid (empty)
 *
 * @return void
 */
public function setPlaceholder($name, $value)
{
    if (empty($name)) {
        throw new InvalidArgumentException('Placeholder $name cannot be empty.');
    }

    $this->placeholders[$name] = $value;
}

/**
 * Adds or updates placeholder(s), passed as an associative array ('name' => value)
 *
 * Name should be a string, value can be any type of value
 *
 * This method is a wrapper for @see addPlaceholder()
 *
 * @param array $placeholders placeholder(s), passed as an associative array ('name' => value)
 *
 * @throws InvalidArgumentException if $placeholders is not an associative array
 *                                  or an empty name is used for a placeholder
 * @return void
 */
public function setPlaceholders(array $placeholders)
{
    if (!is_associative($nameValuePairs)) {
        throw new InvalidArgumentException('Parameter $placeholders is not an associative array');
    }

    foreach (placeholders as $name => $value) {
        $this->addPlaceholder($name, $value);
    }
}


HTML5: Section in Article is Untitled Section in the Outliner Assume I have an article list, each with the following format:

Article

In the header I'm placing: the article's title, the author and the image,
the paragraph belongs to the section, in the footer all the bottom elements (share, read more).

This is my html5 article code:

HTML5 Website

        Title of the Article 1

        Name of the Author

Paragraph content

Title of the Article 2

Name of the Author

Paragraph content

This is the generated outline:
http://gsnedders.html5.org/outliner/

My Questions:
For example1:
Is it benefit to the outliner to mix Title and Author in the same element?
Considering that the outliner reads both as one.

For example2:
The problem I find is that when using section it generates an untitled section in the outliner. So the only solution is to use a div instead of a section. Is that semantic?

General Question:
I want to know if my whole code is properly semantic, respecting both html5 specification and the outliner.

Question 1 (title and author in heading)

  Is it benefit to the outliner to mix Title and Author in the same element? Considering that the outliner reads both as one.

Does the author name needs to be in the article heading?

If so, instead of
Title of the Article 1
Name of the Author
 (the br element is wrong here) you should divide the parts like
TITLE by AUTHOR
 or
TITLE (AUTHOR)
. Use span if you want to style these parts differently.

The title/author combination in your second example is wrong, because it creates a wrong outline. You should use hgroup here:

  <hgroup>

Title of the Article 2

Name of the Author

  </hgroup>

But I'd advise against putting the author name in a heading (depends on the real site context, though). I'd only put the author name in a heading (next to the title or in a hgroup together with the title), if there is no more than one article per author or if the titles are ambiguous.

Question 2 (section vs. div in article)

  The problem I find is that when using section it generates an untitled section in the outliner. So the only solution is to use a div instead of a section. Is that semantic?

Within article you would use section elements for chapters etc. (and other article elements for comments). So yeah, if you don't have/need those chapters (each would have its own heading, whether explicit or implicit = untitled), use div. Or better (if you don't need any special styling hook), don't use any element. Everything within article (which is not within header/footer/nav/'aside') is the "prose" content.

Question 3 (whole markup)

  I want to know if my whole code is properly semantic, respecting both html5 specification and the outliner.

I'd go with:

Title of the Article 1

    Name of the Author

Paragraph content

header is not needed here, because it would only contain the h1
used h1 instead of h2 (the first heading counts, no matter which level; simply use what you like more)
used footer instead of header for the author name and author picture; see HTML5 spec: "A footer typically contains information about its section such as who wrote it …"
placed the "read more" link in nav because it is the main navigation within this sectioning content

you are free to give an explicit heading for nav (and maybe visually hide it, so that it is still read by screen-readers), but it's not a must, of course

whether the social links belong in nav or not depends on context; in general, these are not considered major navigation, so I put them in footer (instead, an aside element could be used)

Regarding @danchet remark about header/footer: It's totally fine (and often needed!) to have several header and footer elements on the same page page. The footer element applies to "its nearest ancestor sectioning content or sectioning root element", so each section/article/nav/aside/etc. can have its very own footer (and yes, even several).


Simple anagram or permutation generator in Python In preparation for a job interview I decided to try and implement the classic find all anagrams of a given string solution. I wanted to see if I could translate a javascript solution into python. This was my attempt:

def anagrams(word):
    if len(word) < 2:
        return word
    else:
        tmp = []
        for i, letter in enumerate(word):
            for j in anagrams(word[:i]+word[i+1:]):
                tmp.append(j+letter)
    return tmp

if __name__ == "__main__":
    print anagrams("abc")

Your python code is neat, simple and easy to understand (so generally it's good); and yet, something in me doesn't like the use of so many temporary arrays.  How about using a generator?

def anagrams(word):
    """ Generate all of the anagrams of a word. """
    if len(word) < 2:
        yield word
    else:
        for i, letter in enumerate(word):
            if not letter in word[:i]: #avoid duplicating earlier words
                for j in anagrams(word[:i]+word[i+1:]):
                    yield j+letter

if __name__ == "__main__":
    for i in anagrams("acbcddd"):
        print i

I (now) filtered out all duplicates which is almost certain your original intention and is standard in all anagram problems.  I did this by seeing if the same letter was already earlier in the word in which case we will have already generated this anagram.

You would have to do the same, or get rid of duplicates in your list for example by doing

return list(set(tmp))

instead of your current return from anagrams.


Automatic condensing of whitespace in HTML One of the things that has occurred to me more recently is that if you add up all of the bandwidth that is taken up by thousands of users downloading the newlines and tabs used to make HTML source code readable... that's a LOT of wasted bandwidth.

So I wanted to add some code that would condense whitespace on-the-fly before sending it to the browser.

I then very quickly realised that sometimes, whitespace is significant. For example, in <textarea>s or <pre> tags.

After double-checking, I've come to the firm conclusion that there is absolutely no use of white-space CSS values such as pre-wrap that would make whitespace significant, except in the case of a custom <rainbow> tag (which is parsed by JavaScript for colour codes and things - long story XD)

So with that in mind, I wanted to condense whitespace that is not contained in a <textarea>, <pre> or <rainbow>. I accomplished this with DOMDocument and a DOMXPath, in other words, "doing it right". But then I learned about regex verbs, and replaced my code with this:

// uber hax with regex
$str = trim(preg_replace("(<(textarea|rainbow|pre).*?</\\1>(*SKIP)(*F)|\s+)s"," ",$str));

It, um... well, it works, or at least it seems to from what I can tell. But I wanted to post here so I can ask, have I missed anything important? Or is it really this simple?

I don't see much reason to use (*F) instead of the somewhat more understandable (*FAIL).
Consider using single-quoted (') strings for regular expressions to avoid \\1.
There's a big assumption here that the HTML will be well-formed. I assume (ha!) that this is okay.
Whitespace is significant in <input> tags.


Small module for getting JSON to work with in Python I currently have a module with Class / Methods I want to use in other scripts. Recently chess.com released a read-only REST API for beta-testing. The methods in my class build a URL for the API endpoints I am interested in and get the JSON data for me to work with in other scripts.

It is worth noting that I am fairly unused to using classes in the way I am in my code below - or at all really. Here is my working code:

"""TODO: docstring"""
from requests import get

class GetGamesJson:
    """TODO: docstring"""

    def build_url_for_player(self, username):
        """Build a custom URL for the `Palyer Profile` endpoint.

        Args:
            username (`str`): Player's username as string input.

        Returns:
            string: A URL for a player that can be used as a base for
            accessing other enpoints.

        Examples:
            >>> print(build_url_for_player('foo'))
            'https://api.chess.com/pub/player/foo'

        Raises:
            ValueError: If `username` is not found.

        """
        self.username = username
        player = (
            "https://api.chess.com/pub/player/{username}".format(
                username=username
            )
        )
        try:
            if get(player).json()['code'] == 0:
                raise ValueError("User '{username}' not found.".format(
                    username=username
                ))
        except KeyError:
            return player

    def build_url_for_monthly_archives(self, year, month):
        """Build URL for the `Monthly Archives` endpoint.

        This method depends on the base URL returned by the
        build_url_for_player() method.

        Args:
            year (int): Year as integer input.
            months (int): Month as integer input.

        Returns:
            string: URL to Monthly Archives.

        Examples:
            >>> print(build_url_for_monthly_archives(2009, 5))
            'https://api.chess.com/pub/player/foo/games/2009/05'

        """
        self.year = year
        self.month = month
        username = self.username
        url_to_monthly_archive = (
            self.build_url_for_player(username) +
            "/games/{year}/{month:02d}".format(
                username=username, year=year, month=month
            )
        )
        return url_to_monthly_archive

    def get_monthly_archive_json_doc(self):
        """Returns a JSON document from Monthly Archives endpoint."""
        link_to_json_doc = self.build_url_for_monthly_archives(
            self.year, self.month
        )
        get_json_doc = get(link_to_json_doc)
        return get_json_doc.json()

I would like to know what I can do to simplify and improve in the code I have.

I think you are over-complicating the problem, I would apply the following modifications:

rename the class to, say, ChessAPI
pass username into the class constructor
check for username to exist and define the profile_url instance attribute inside the class constructor
instantiate a single Session object and re-use inside the class methods

Modified code:

"""TODO: docstring"""
import requests

class ChessAPI:
    """TODO: docstring"""
    def __init__(self, username):
        self.username = username

        self.session = requests.Session()

        self.profile_url = "https://api.chess.com/pub/player/{username}".format(username=self.username)
        try:
            if self.session.get(self.profile_url).json()['code'] == 0:
                raise ValueError("User '{username}' not found.".format(username=self.username))
        except KeyError:
            pass

    def get_monthly_archives(self, year, month):
        """Returns a JSON document from Monthly Archives endpoint."""
        archive_url = self.profile_url + "/games/{year}/{month:02d}".format(username=self.username, year=year, month=month)
        get_json_doc = self.session.get(archive_url)
        return get_json_doc.json()

Sample usage (works for me):

api = ChessAPI('alex')
archives = api.get_monthly_archives(2017, 1)
print(archives)


Rails thin controller method to change account plan and fire slack message via a service object I'm a beginner with Rails and am trying to figure out how to keep my controllers clean and thin, and how to work with service objects. I am building an internal tool to manage our SaaS customers, and the specific feature I'm building is to allow admins to change a customer's plan.

In accounts_controller.rb, I have the following method:

def change_plan
  account = Account.find(params[:id])

  old_plan = account.plan
  new_plan = Plan.find(params[:account][:plan_id])

  account.change_plan!(params[:account][:plan_id])

  SlackService.new.plan_change(current_user, account, old_plan, new_plan).deliver
  redirect_to action: "show", id: params[:id]
end

My first question is around the lines 2 and 3 of the method. Is there some place I should be moving this logic? Some sort of SlackService container object? Or should the SlackService method itself be handling the retrieval of the old_plan and new_plan? I realize this is fairly trivial but would love to build it properly.

Here's the code for the SlackService itself, heavily inspired by this article:

require 'net/http'

class SlackService
  NAME_AND_ICON = {
    username: 'Dashboard',
    icon_emoji: ':bat:'
  }

  SLACK_WEBHOOK_URL =  "https://hooks.slack.com/services/###/###/###"
  SLACK_WEBHOOK_CHANNEL = Rails.env.production? ? "###" : "###"

  def initialize(channel = SLACK_WEBHOOK_CHANNEL)
    @uri = URI(SLACK_WEBHOOK_URL)
    @channel = channel
  end

  def plan_change(user, account, old_plan, new_plan)
    params = {
      attachments: [
          {
            author_name: "#{user.first_name} #{user.last_name}",
            text: "Account: #{account.name} (#{account.id})",
            title: 'Dashboard Plan Change',
            fields: [
              {
                title: 'Old Plan',
                value: "#{old_plan.name} (#{ActionController::Base.helpers.number_to_currency(old_plan.price)})",
                short: true
              },
              {
                title: 'New Plan',
                value: "#{new_plan.name} ($#{ActionController::Base.helpers.number_to_currency(new_plan.price)})",
                short: true
              },
            ]
          }
        ]
    }
    @params = generate_payload(params)
    self
  end

  def deliver
    begin
      Net::HTTP.post_form(@uri, @params)
    rescue => e
      Rails.logger.error("SlackService: Error when sending: #{e.message}")
    end
  end

  private

  def generate_payload(params)
    {
      payload: NAME_AND_ICON
        .merge(channel: @channel)
        .merge(params).to_json
    }
  end
end

It seemed that I had to require net/http here in the service, but wasn't sure if that was the proper place for it.
I think the SLACK_WEBHOOK_URL and SLACK_WEBHOOK_CHANNEL constants should be defined here.
I see many examples of services where the methods are class level, but it seemed to make sense to keep it instance level so you can define the channel on instantiation.
Tying in with the above, the plan_change method accepts 4 params, but not sure if there's a cleaner way.
Wasn't sure if my approach to loading the NumberHelper is correct.

def change_plan
  account = Account.find(params[:id])

  old_plan = account.plan
  new_plan = Plan.find(params[:account][:plan_id])

  # maybe this method ought to return the new_plan, since it's basically repeating the line above
  account.change_plan!(params[:account][:plan_id])

  # since account has plan method on you don't necessarily need to pass the `new_plan` explicitly
  SlackService.new.plan_change(current_user, account, old_plan, new_plan).deliver
  redirect_to action: "show", id: params[:id]
end

Now to address your questions

  It seemed that I had to require net/http here in the service, but wasn't sure if that was the proper place for it.

That's absolutely fine.

  I think the SLACK_WEBHOOK_URL and SLACK_WEBHOOK_CHANNEL constants should be defined here.
  These could be defined in a config file.
  I see many examples of services where the methods are class level, but it seemed to make sense to keep it instance level so you can define the channel on instantiation.

It's fine to use instance methods. However, you can go one step further and define something like

def SlackService.plane_change(current_user:, account:, old_plan: new_plan:)
  SlackService.new.plan_change(current_user, account, old_plan, new_plan).deliver
end

The nice thing about this is in your controller, you only have to do SlackService.plane_change(current_user:, account:, old_plan: new_plan:) this in the contoller, and you rid the controller of instantiating the class, calling method etc.

  Tying in with the above, the plan_change method accepts 4 params, but not sure if there's a cleaner way.

4 is just about ok (see Sandi Metz's article https://robots.thoughtbot.com/sandi-metz-rules-for-developers#four-method-arguments)

  Wasn't sure if my approach to loading the NumberHelper is correct.

I am not sure I understand this question


Fetching the total amount of SE reputation accumulated Since I am new to R, I have pulled this code together in kind of a rag-tag way, but I am wondering, is there something similar to list comprehensions (in Python) I can use in R to make this simpler? Or a better way of doing this? I am trying to fetch the total amount of reputation a user has accumulated on Stack Exchange.  I am ideally looking for a way to remove the for loop and use the sum function on a subset of items (from the API response).

library(httr)
s = 0
for(i in content(GET(paste("http://api.stackexchange.com/users/",readline(),"/associated",sep="")))$items) {
  q=i$reputation
  if (q>101)
    s=s+q
}
print(s)

Sample input would be a user's id, like 10400443.

It's simple to extract all reputation and subsequently sum an index. This is instead of performing the ifelse inside the *apply function to demonstrate indexing.

library(httr)
allc <- content(GET(paste0("http://api.stackexchange.com/users/",'10400443',"/associated")))
allci <- allc$items

# get all reputation out of list
allrep <- sapply(allci,`[[`,'reputation')

# sum only index of > 101
sum(allrep[allrep > 101])

I'm not familiar with Python, but it appears that indexing in R serves the same purpose as list comprehension in Python - list comprehension in r


Node PSP ISO Scraper I recently bought a PSP and wanted to know the best ISO files and wrote a scraper to retrieve games ISOs titles that received a high rating and send them to a csv. Any recommendations as to readability, and/or better code optimizations?

var request = require('request');
var cheerio = require('cheerio');
var json2csv = require('json2csv');
var fs = require('fs');

var url = 'http://goo.gl/cc4HRc';
var pspISOs = [];

request(url, function (error, response, html) {
  if (!error && response.statusCode === 200) {
    var $ = cheerio.load(html);
    $('.gamelist').each(function () {
    var titles = $(this).text();
    var ratings = +parseFloat($(this)
      .parents('tr').find('td')
      .last().text()).toFixed(2);
    pspISOs.push({ title: titles, rating: ratings });
  });
  var topISOs = pspISOs.filter(function (iso) {
    return iso.rating >= 4.81;
  });
  var columns = Object.keys(topISOs[0]);
  json2csv({ data: topISOs, fields: columns }, function (err, csv) {
    if (err) console.log(err);
      fs.writeFile('psp_iso.csv', csv, function (err) {
      if (err) throw err;
        console.log('Saved.');
      });
    });
  }
});

After revisiting this, and even though I prefer to handle tasks like this with node, it is possible to write a bash script using curl, and/or xpath that accomplishes this same task. The ratings could filtered with a regex pattern within grep just to lay down a few ideas. Again, this is just an alternative solution.


Class with multiple workers I am building a tool that interacts with a batched stream of incoming data. This data needs to be processed and the result returned. To split up the work I have created a class that has inbound (_in) and outbound (out) queues and workers that are getting, processing, and depositing the work.
This example takes an iterable of numbers (in pass_data) and multiplies them by f.
import queue, random, time
from multiprocessing import Process, Queue

def _worker(_in, out, f):
    """Get work from _in and output processed data to out"""
    while True:
        try:
            work = _in.get()
        except queue.Empty:
            continue
        # simulate blocking for some time
        time.sleep(random.uniform(0.01, 0.5))
        out.put(work * f)

class C:
    def __init__(self, f, threads=2):
        self.f = f
        self.threads = threads
        self._in, self.out = Queue(), Queue()
        self.args = (self._in, self.out, self.f)
        self.workers = [
            Process(target=_worker, args=self.args) for _ in range(self.threads)
        ]

    def __repr__(self):
        return f"{self.__class__.__name__}(threads={self.threads})"

    def start(self):
        """Start all workers"""
        for worker in self.workers:
            worker.start()

    def terminate(self):
        """Terminate all workers"""
        for worker in self.workers:
            worker.terminate()

    def pass_data(self, data):
        """Pass data to the queue to be processed"""
        for rec in data:
            self._in.put(rec)

    def get_completed(self):
        """Return a list of processed data"""
        items = []
        while True:
            try:
                items.append(self.out.get_nowait())
            except queue.Empty:
                break
        return items

if __name__ == "__main__":
    c = C(f=12, threads=2)
    c.start()

    for i in range(5):
        s = 0
        n = random.randint(1, 20)
        c.pass_data(list(range(n)))
        print(f"sent: {n}")
        while s < n:
            r = c.get_completed()
            s += len(r)
            if r:
                print(len(r), end=", ")
            time.sleep(random.uniform(0.01, 0.4))
        print()
    c.terminate()

This is, at the moment, a proof of concept. Are there any pitfalls to this method? Is there a better way to do this already?!
Aspects that I intend to address:

queue size limits
thread number limits

Use multiprocessing.Pool
The multiprocessing library already has a worker pool implementation ready to be used. Your code could be rewritten as:
import time
from multiprocessing import Pool

def f(x):
    time.sleep(random.uniform(0.01, 0.5))
    return x * 12

if __name__ == "__main__":
    c = Pool(2)

    for i in range(5):
        n = random.randint(1, 20)
        r = c.map_async(f, list(range(n)))
        print(f"sent: {n}")
        print(f"got: {len(r.get())}")

While multiprocessing.Pool allows you to check if the results are ready by using .ready() on the result of an apply_async() or map_async() call, you can't get a partial result from map_async(). However, if you do want to process individual results as soon as they are ready, you can consider calling apply_async() with a callback function that handles the result.


Which is better for checking previous object in list? I have a for loop which loops through a list and builds and object, I needed to check the previous object with the one i'm building...which way is better?

This way?:

    String lastDirection= "";
    List<Stages> stages = new ArrayList();
    for (Object object : results) {
        Vector result = (Vector) object;
        Stages stage = new Stages();
        stage.setStage(result.get(0).toString());
        stage.setDirection((String) result.get(1));
        stage.setStageName((String) result.get(2));
        if(!stage.getDirection.equals(lastDirection))
            stages.add(stage);
        lastAtco = stage.getDirection();
    }
    return stages;

Or This way?:

    List<Stages> stages = new ArrayList();
    for (Object object : results) {
        Vector result = (Vector) object;
        Stages stage = new Stages();
        stage.setStage(result.get(0).toString());
        stage.setDirection((String) result.get(1));
        stage.setStageName((String) result.get(2));
        if (stages.size() >= 1 && stages.get(stages.size() - 1).getDirection().equals(stage.getDirection())) {
        } else {
            stages.add(stage);
        }

    }
    return stages;

I can see how one looks better than the other...but which one would be more efficient?

The former would definitely be more efficient.

In the latter case, you "forget" a local variable that you have available (by letting it go out of scope), and then on the next iteration get it back by calling stages.get().  There's no way that the get() call will be as cheap as simply referencing the lastDirection variable.


Calculating change in Ada I have written a change calculator using Ada. when the user inputs an amount of money it returns a list of the coins and bills needed to create that total. It starts off by trying to use the largest money types then tries smaller ones.

I am still learning how to use Ada and would like to know how i can make better use of the language features. Am i using the right data types, is there a way to make the change calculation loop more readable?

with Ada.Text_IO;
with Ada.Float_Text_IO;
with Ada.IO_Exceptions;
with Ada.Integer_Text_IO;
use Ada;

procedure Change_Calculator is
   type Money is delta 0.01 digits 10;
   Input_Amount : Money := 0.0;
   package Money_IO is new Text_IO.Decimal_IO(Money);

   type Currency_Denomination is record
      Name : String(1..10);
      Value : Money;
   end record;

   Currency_Names  : array (1..10) of Currency_Denomination;
   Currency_Counts : array (1..10) of Integer := (others=>0);
   Currency_Index  : Integer range 1..10;

   procedure Get_Money_Prompt(Amount: out Money) is
      Response : String(1..20);
      Last     : Natural;
   begin
      loop
         declare
         begin
            Text_IO.Put("amount: ");
            Text_IO.Flush;
            Text_IO.Get_Line(Response, Last);
            -- try to convert string input to money type
            Amount := Money'Value(Response(1 .. Last));
            -- quit the loop if the money converted -- '
            exit;
         exception
            when Constraint_Error =>
               Text_IO.Put_Line("ERROR: bad money format");
         end;
      end loop;
   end Get_Money_Prompt;

begin
   Currency_Names := (("Penny     ", 0.01),
                      ("Nickle    ", 0.05),
                      ("Dime      ", 0.10),
                      ("Quarter   ", 0.25),
                      ("Dollar    ", 1.00),
                      ("5 Dollar  ", 5.00),
                      ("10 Dollar ", 10.00),
                      ("20 Dollar ", 20.00),
                      ("50 Dollar ", 50.00),
                      ("100 Dollar", 100.00));

   -- read money amount from the user
   Get_Money_Prompt(Input_Amount);

   -- calculate needed currency
   for I in Currency_Names'Range loop
      -- calculate inverse of index to count backwords
      Currency_Index := (Currency_Names'Length-I)+1;
      while Input_Amount >= Currency_Names(Currency_Index).Value loop
         Input_Amount := Input_Amount - Currency_Names(Currency_Index).Value;
         -- increment currency useage in array
         Currency_Counts(I) := Currency_Counts(I) + 1;
      end loop;
   end loop;

   -- display needed currency
   for I in Currency_Counts'Range loop
      Currency_Index := (Currency_Names'Length-I)+1;
      -- do not display unused currency
      if Currency_Counts(Currency_Index) /= 0 then
        Text_IO.Put(Currency_Names(I).Name & " ");
        Integer_Text_IO.Put(Currency_Counts(Currency_Index));
        Text_IO.New_Line;
      end if;
   end loop;

end Change_Calculator;

Currency_Names, Currency_Counts and Currency_Index all use the same range,
so it is better to declare a ranged type (or a subtype):

   type Index is range 1..10;
   Currency_Names  : array (Index) of Currency_Denomination;
   Currency_Counts : array (Index) of Integer := (others=>0);
   Currency_Index  : Index;

To loop backwards, it is better to use the reverse keyword:

   -- calculate needed currency
   for I in reverse Currency_Names'Range loop
      while Input_Amount >= Currency_Names(I).Value loop
         Input_Amount := Input_Amount - Currency_Names(I).Value;
         -- increment currency useage in array
         Currency_Counts(I) := Currency_Counts(I) + 1;
      end loop;
   end loop;

   -- display needed currency
   for I in Currency_Counts'Range loop
      -- do not display unused currency
      if Currency_Counts(I) /= 0 then
        Text_IO.Put(Currency_Names(I).Name & " ");
        Integer_Text_IO.Put(Currency_Counts(I));
        Text_IO.New_Line;
      end if;
   end loop;

A nice side-effect is that the index of Currency_Counts will now correspond to the index of Currency_Names, no need to calculate the index anymore, and the Currency_Index variable is no longer needed.


Replacing a string until it matches a certain length I'm trying to make an SMS Shortner for a kata at Codewars:

  Your task is to shorten the message to 160 characters, starting from end, by replacing spaces with camelCase, as much as necessary.

I've tried with a while loop and although the logic is correct, it's taking too long to process my tests. How can I optmize this and what parts are taking longer than it could?

const shortener = (message) => {
  let r = message.split('').reverse().join('');
  while (r.length > 160) {
    r = r.replace(/([^\s]\s)/, (m,s) => s.trim().toUpperCase());
  }
  return r.split('').reverse().join('');
}

That is an O(L2) algorithm: each replace involves copying the entire string. A better strategy would be to call replace just once. First, figure out how many spaces need to be eliminated. Then, use a callback that conditionally modifies its match based on a count.

It should be possible to do it that way without reversing the string twice.


method to decide if two strings are anagrams I'm studying with the Cracking the Code Interview book. I'm just looking for some feedback on this method:

C++

bool areAnagrams(const char*s1, const char*s2){
    unordered_map <char,int> m1,m2;

    for(size_t i = 0; i < strlen(s1); i++){
        m1[s1[i]] += 1;
    }
    for(size_t j = 0; j < strlen(s2); j++){
        m2[s2[j]] += 1;
    }
    for(auto i = m1.begin(); i != m1.end(); i++){
        if(m2[(*i).first] != (*i).second)
            return false;
    }
    return true;
}

Bug
Your algorithm is not quite correct.  For example:
areAnagrams("abc",  "abc")  -> true  (correct)
areAnagrams("abc",  "abcd") -> true  (wrong)
areAnagrams("abcd", "abc")  -> false (correct)

The problem is that you are checking that the characters in s1 exist with the same frequency in s2.  But there can be characters in s2 that do not exist in s1 and you don't check for those.
An easy way to remedy this would be to check that both strings have the same length.  This plus the frequency check you are already doing should ensure that the strings are anagrams.


SPOJ problem - The last digit of a number to a power Here is the problem statement:

  Given integers \$a\$ (\$0 \le a \le 20\$) and \$b\$ (\$0 \le b \le 2,147,483,000\$), where \$a\$ and
  \$b\$ are not both 0, find the last digit of \$a^b\$.

  Input

  The first line of input contains an integer \$t\$, the number of test
  cases (\$t \le 30\$). \$t\$ test cases follow. For each test case will appear \$a\$
  and \$b\$ separated by a space.

  Output

  For each test case output an integer per line representing the result.

  Example Input:

2
3 10
6 2

  Example Output:

9
6

Here is the code which is exceeding the time limit:

#include <stdio.h>

int main()
{
    int t;
    scanf("%d",&t);
    while(t--)
    {
        long long int base,exponent;
        scanf("%lld%lld",&base,&exponent);
        if(base==0&&exponent==0)
        {
            printf("1\n");
        }
        else
        {
            long long int digit=1;
            long long int i;
            for(i=1;i<=exponent;i++)
            {
                digit=(base*digit)%10;
            }
            printf("%lld\n",digit);
        }
    }
    return 0;
}

How do I make this more efficient?

As you have probably noticed, this is not efficient because you have a loop running exponent times (which could be over 2 billion). The way to make this more efficient is not to make your code faster, but to choose a better algorithm.

I'm not going to tell you outright what the answer is, but do the following. Add some output to your loop:

        for(i=1;i<=exponent;i++)
        {
            digit=(base*digit)%10;
            printf("digit = %d\n", digit);
        }

and then run your program for some (small) sample inputs. You should notice a pattern. The key is to identify the pattern and use it to avoid running the loop a billion times.


Reading a line ending with a newline character from a file descriptor I'm implementing a function that behaves like getline() which reads a line from file descriptor and returns the results without \n. allowed functions, read(), free() and malloc. Please review my code.

#include <unistd.h>
#include <stdlib.h>
#include <string.h>
#ifndef GET_C_BUFF_SIZE
# define GET_C_BUFF_SIZE 1023
#endif
#ifndef BUFF_SIZE
# define BUFF_SIZE 32
#endif

void    *my_memcpy(void *dest, const void *src, size_t n)
{
    unsigned char       *n_dest;
    const unsigned char *n_src;

    n_dest = (unsigned char *)dest;
    n_src = (unsigned char *)src;
    while (n--)
        *n_dest++ = *n_src++;
    *(++n_dest) = '\0';
    return (dest);
}

void    *my_realloc(void *ptr, size_t len)
{
    void    *real;

    real = malloc(len);
    if (real)
        my_memcpy(real, ptr, len);
    free(ptr);
    return (real);
}

int my_getchar(const int fd)
{
    static char buff[GET_C_BUFF_SIZE];
    static char *chr;
    static int  pos = 0;
    static int  ret = 0;

    if (pos >= ret)
    {
        if ((ret = read(fd, buff, GET_C_BUFF_SIZE)) > 0)
        {
            chr = buff;
            pos = 0;
            return (*(chr + pos++));
        }
        else
            return (0);
    }
    else
        return (*(chr + pos++));
}

int read_line(char *text, int buf_size, char **line, const int fd)
{
    int     position;
    int     c;

    position = 0;
    while (1)
    {
        c = my_getchar(fd);
        if (c == 0 || c == '\n')
        {
            text[position] = '\0';
            *line = text;
            return (1);
        }
        else
            text[position] = c;
        position++;
        if (position >= buf_size)
        {
            buf_size += BUFF_SIZE;
            text = my_realloc(text, buf_size);
            if (!text)
                return (-1);
        }
    }
    return (1);
}

int get_line(const int fd, char **line)
{
    char    *text;
    int     buf_size;

    buf_size = BUFF_SIZE;
    text = malloc(sizeof(char) * buf_size);
    if (fd < 0 || !text || !line)
        return (-1);
    return (read_line(text, buf_size, line, fd));
}

Static buffer

In my_getchar(), you use a static buffer buff along with some other static variables.  The problem with this is that you can no longer call get_line() on two different files at the same time.  For example, if you did this:

get_line(fd1, &line1);
get_line(fd2, &line2);

Then the second get_line() would actually read characters from the first file descriptor, because you have buffered its contents in the static buffer.

Reallocation strategy

Your current reallocation strategy is to increase the size of the buffer by BUFF_SIZE when you run out of space.  This causes your function to have a \$O(n^2)\$ time complexity.  It would be better if you doubled the size of your buffer instead of adding a constant to it.


Creating a simple login with AngularJS I'm brand new to AngularJS. I would like advice as to whether I'm approaching the design of a simple login section of an app built with Angular correctly.

The app consists of two view partials: login.html and user-admin.html. Of what I have so far, the user types their username into the login page. A controller checks if that username is listed in users.json file, if so then login is successful and the user-admin.html partial replaces login.html.

I feel that the controller which checks the typed username against the usernames in users.json could be written better. Is there a more efficient way than using the 'for' statement?

To develop this further I will want to add some way of preventing a logged in user seeing the login page and a non-logged in user seeing the admin page. My initial thought is to use cookies. But would the 'Angular' way be to create a service that perpetuates the logged in status between views? If that is the best way to implement it, what would that service look like?

app.js

angular.module('userApp', ["ngResource"]).

config(['$routeProvider', function($routeProvider) {
    $routeProvider.
        when('/login', {templateUrl: 'partials/login.html',   controller: LoginCtrl}).
        when('/loggedin', {templateUrl: 'partials/user-admin.html', controller: UserCtrl}).
        otherwise({redirectTo: '/login'});
}],[ '$locationProvider', function($locationProvider) {
    $locationProvider.html5Mode = true;
}]).

factory("User", function($resource) {
    return $resource("users/:userId.json", {}, {
        query: {method: "GET", params: {userId: "users"}, isArray: true}
    });
});

controllers.js

function LoginCtrl($scope, $route, $routeParams, $location, User) {
    $scope.users = User.query();
    $scope.loginUser = function() {
        var loggedin = false;
        var totalUsers = $scope.users.length;
        var usernameTyped = $scope.userUsername;

        for( i=0; i < totalUsers; i++ ) {
            if( $scope.users[i].name === usernameTyped ) {
                loggedin = true;
                break;
            }
        }

        if( loggedin === true ) {
            alert("login successful");
            $location.path("/loggedin");
        } else {
            alert("username does not exist")
        }
    }
}

function UserCtrl($scope, $route, $routeParams, $location) {
    $scope.logoutUser = function() {
        $location.path("/login");
    }
}

users.json

[
    {
        "userId": 1,
        "name": "Tommy",
        "password": "123456",
        "log": {
            "registration": "2012.12.14",
            "lastLog": "2013.01.15"
        }
    },
    {
        "userId": 2,
        "name": "Anne",
        "password": "123456",
        "log": {
            "registration": "2012.12.24",
            "lastLog": "2012.12.29"
        }

    },
    {
        "userId": 3,
        "name": "Miles",
        "password": "abc",
        "log": {
            "registration": "2013.02.01",
            "lastLog": "2013.02.01"
        }
    }
]

login.html

Login

    <form ng-submit="loginUser();">
        Username<input type="text" id="userUsername" ng-model="userUsername">
        <input type="submit" value="Login">
    </form>

There is much wrong with your code..

You are iterating over every record to find a username, what happens if you have lots of users ?
You are not verifying the password !?
It seems like you are exposing the REST services with all the user id's and passwords !?
It seems that if the user knows the /loggedin URL, then the user can skip logging in since you do not validate loggedIn anywhere. Even worse, loggedIn is a variable local to LoginCtrl.

It is good to try and write everything yourself, it brings valuable experience. But please, use this only for throwaway websites.


Function to erase original element but also linked I have a list of pointers to objects where some of the objects contain a pointer to another object.  When I remove an object in the list which has a linked object I need to remove them BOTH.  I also want to position the next iterator at a convenient position which will usually be just after the object in the list which is to be deleted.  Or if the linked item is next it will have to be just after the linked item.

There are peripheral issues such as why not using smart pointers etc.  But this is just a simple demo - so please ignore lifecycle issues.  I am looking for comments on the RemoveAllAssociated function.

   #include <iostream>
   #include <list>
   #include <algorithm>

   using namespace std;

   class A
   {
   public:
      A(int timestamp) : m_timestamp(timestamp), m_red(false), m_linkedmsg(0) {}
      virtual ~A() {}

      int gettimestamp() const { return m_timestamp; }

      A* getLinkedMsg() const { return m_linkedmsg; }
      void setLinkedMsg(A* linked) { m_linkedmsg = linked; }

      bool IsRed() const { return m_red; }
      void setRed(bool col = true) { m_red = col; }

   protected:
      int m_timestamp;
      bool m_red;
      A* m_linkedmsg;
   };

   class B : public A
   {
   public:
      B(int timestamp) : A(timestamp) { setRed(true); }
      virtual ~B() {}

   };

   //erase item and linked iterator if available and return next iterator
   std::list<A*>::iterator RemoveAllAssociated(std::list<A*>& thelist, A* item) {
      std::list<A*>::iterator it = find(thelist.begin(), thelist.end(), item);
      if(it != thelist.end()) {
         cout << "found original item in list with timestamp=" << (*it)->gettimestamp() << "\n";
         //we now want to delete linked as well as original
         //first delete linked
         A* linked = (*it)->getLinkedMsg();
         std::list<A*>::iterator found_it = find(thelist.begin(), thelist.end(), linked);
         if(found_it != thelist.end()) {
            cout << "found linked item with timestamp=" << (*found_it)->gettimestamp() << "\n";
            delete *found_it;
            std::list<A*>::iterator after_linked_it = thelist.erase(found_it);
         }

         //now erase item in list
         //Need to return next iterator.  Usually will be just after item
         //check a valid iterator returned
         //We want iterator to be positioned either just after
         delete *it;  //or could have done: delete item;
         return thelist.erase(it);
      } else
         cout << "original item not found in list\n";

      return thelist.end();
   }

   int main() {

      A* a1 = new B(0);
      A* a2 = new B(1);
      A* a3 = new B(2);
      A* a4 = new B(3);
      A* a5 = new B(4);
      A* a6 = new B(5);

      a2->setLinkedMsg(a1);

      //put them in a list
      std::list<A*> alist;
      alist.push_back(a5);
      alist.push_back(a6);
      alist.push_back(a1);
      alist.push_back(a2);
      alist.push_back(a3);
      alist.push_back(a4);

      std::list<A*>::iterator it = RemoveAllAssociated(alist, a2);
      if(it != alist.end())
         cout << "next item in list is " << (*it)->gettimestamp() << endl;

      //print out what is left to check correct items erased
      for(it = alist.begin(); it != alist.end(); ++it) {
         cout << "address=" << *it << " ts=" << (*it)->gettimestamp() << endl;
      }

      //cleanup
      it = alist.begin();
      while(it != alist.end()) {
         delete *it;
         it = alist.erase(it);
      }

      return 0;
   }

OK. This is NOT C++ code. This is C that happens to use a few C++ features like std::list. I would call this "C with classes".

What you have failed to do is define the ownership of the pointers. Without ownership calling delete is a pain (because you don't know if calling delete is a good idea because you don't know the ownership).

There is no point in reviewing further until you define ownership semantics.
Who owns the pointer.

Is it shared ownership.
Is it exclusive ownership.
Are there cycles?

This looks really awful:

  A* a1 = new B(0);
  A* a2 = new B(1);
  A* a3 = new B(2);
  A* a4 = new B(3);
  A* a5 = new B(4);
  A* a6 = new B(5);

You have 6 RAW pointers. What happens if the constructor to B(3) throws an exception. Then you have just leaked a1/a2/a3. In modern C++ it is very rare to see RAW pointers. Pointers are usually contained inside an object (usually a smart pointer).

Now you are passing the pointers to a list.

  //put them in a list
  std::list<A*> alist;
  alist.push_back(a5);
  alist.push_back(a6);
  alist.push_back(a1);
  alist.push_back(a2);
  alist.push_back(a3);
  alist.push_back(a4);

So now you have two people pointing at each object. So who is supposed to destroy them? You have not defined the ownership semantics thus we do not know who is responsible for deleting the pointers. Even worse if they are deleted via one method the other pointer points at an invalid location (as there is no feedback) between the pointers.

boost::ptr_list would hold and take ownership of the pointers very nicely.

Cleanup outside a destructor!!!

  //cleanup
  it = alist.begin();
  while(it != alist.end()) {
     delete *it;
     it = alist.erase(it);
  }

This is not exception safe. You need to use RAII to make sure you can never leak resources even in the presence of pointers.


Create a two-dimensional Gaussian kernel In my code I have a function that generate a 2D gaussian function given sigma like so:

#include <opencv2/highgui.hpp>
#include <opencv2/imgproc.hpp>

#include <iostream>
int main(int argc, char* argv[])
{
  cv::Mat kernel = getGaussianKernel(rows, cols, 50, 50 );
  //Then I use that kernel for some processing

  //return 0;
}
cv::Mat getGaussianKernel(int rows, int cols, double sigmax, double sigmay )
    {
        cv::Mat kernel = cv::Mat::zeros(rows, cols, CV_32FC1);

        float meanj = (kernel.rows-1)/2,
              meani = (kernel.cols-1)/2,
              sum = 0,
              temp= 0;

        int sigma=2*sigmay*sigmax;
        for(unsigned j=0;j<kernel.rows;j++)
            for(unsigned i=0;i<kernel.cols;i++)
            {
                temp = exp( -((j-meanj)*(j-meanj) + (i-meani)*(i-meani))  / (sigma));
                if (temp > eps)
                    kernel.at<float>(j,i) = temp;

                sum += kernel.at<float>(j,i);
            }

        if(sum != 0)
            return kernel /= sum;
        else return cv::Mat();
    }

That function takes, after profiling, ~14% of my time and sigma parameter for most of the cases is constant, also rows and cols don't change because I'm using a camera and its resolution won't change.

I want to know if there is a way to use that kernel without wasting 14% of the time or to optimize its function?

I tried to save all the data generated in a file (.yml .txt 13mb) and read them but it takes the same time or little bit more

The two-dimensional Gaussian function can be obtained by composing two one-dimensional Gaussians.

I changed your code slightly so that it would compile (and not optimize away the unused kernel):

#include <iostream>
int main()
{
    int rows = 20000, cols = 20000;
    const auto kernel = getGaussianKernel(rows, cols, 50, 50 );
    std::cout << kernel.total() << std::endl;
}

This runs in

3.90user 0.38system 0:04.29elapsed

Changing to precompute separate x and y Gaussian curves gives a 10x improvement in speed:

#include <opencv2/core/core.hpp>
#include <cmath>

cv::Mat getGaussianKernel(int rows, int cols, double sigmax, double sigmay)
{
    const auto y_mid = (rows-1) / 2.0;
    const auto x_mid = (cols-1) / 2.0;

    const auto x_spread = 1. / (sigmax*sigmax*2);
    const auto y_spread = 1. / (sigmay*sigmay*2);

    const auto denominator = 8 * std::atan(1) * sigmax * sigmay;

    std::vector<double> gauss_x, gauss_y;

    gauss_x.reserve(cols);
    for (auto i = 0;  i < cols;  ++i) {
        auto x = i - x_mid;
        gauss_x.push_back(std::exp(-x*x * x_spread));
    }

    gauss_y.reserve(rows);
    for (auto i = 0;  i < rows;  ++i) {
        auto y = i - y_mid;
        gauss_y.push_back(std::exp(-y*y * y_spread));
    }

    cv::Mat kernel = cv::Mat::zeros(rows, cols, CV_32FC1);
    for (auto j = 0;  j < rows;  ++j)
        for (auto i = 0;  i < cols;  ++i) {
            kernel.at<float>(j,i) = gauss_x[i] * gauss_y[j] / denominator;
        }

    return kernel;
}

0.32user 0.36system 0:00.69elapsed

(both versions compiled with GCC 7.1.0 with -O3 -march=native on an Intel i7-Q6700)

The calculations are reasonably independent, so OpenMP may help.  But when I tried, it always worked out slower - probably too much false sharing for it to be useful.

Note that my code doesn't initialize the kernel with zeros - it leaves it uninitialized, and instead writes to every element when it reaches it.  This saves two passes over the memory.  I precompute the denominator for the same reason, though that's only truly valid for a kernel of infinite size.

Also note that I changed the calculation of mid-point to divide by 2.0 rather than by 2 - I wasn't sure whether the truncation was intentional or not.

A further speed increase is gained by using OpenCV to work on a row at a time:

cv::Mat getGaussianKernel(int rows, int cols, double sigmax, double sigmay)
{
    auto gauss_x = cv::Mat_<float>(cols, 1);

    const auto x_mid = (cols-1) / 2.0;
    const auto y_mid = (rows-1) / 2.0;

    const auto x_spread = 1. / (sigmax*sigmax*2);
    const auto y_spread = 1. / (sigmay*sigmay*2);

    for (auto i = 0;  i < cols;  ++i) {
        auto const x = i - x_mid;
        gauss_x[i][0] = std::exp(-x*x * x_spread);
    }

    auto kernel = cv::Mat_<float>(rows, cols);
    for (auto i = 0;  i < rows;  ++i) {
        auto const y = i - y_mid;
        kernel.row(i) = gauss_x * std::exp(-y*y * y_spread);
    }

    const auto denominator = std::accumulate(kernel.begin(), kernel.end(), 0);

    return kernel / denominator;
}

0.17user 0.00system 0:00.17elapsed

This is also shorter, and arguably easier on the eye.  In this version, I show an alternative to precomputing the denominator - use std::accumulate to add it together afterwards.

The simplest and fastest version, though, is to use the Gaussian distribution provided by OpenCV, and just matrix-multiply the two together:

#include <opencv2/core/core.hpp>
#include <opencv2/imgproc/imgproc.hpp>

cv::Mat getGaussianKernel(int rows, int cols, double sigmax, double sigmay)
{
    auto gauss_x = cv::getGaussianKernel(cols, sigmax, CV_32F);
    auto gauss_y = cv::getGaussianKernel(rows, sigmay, CV_32F);
    return gauss_x * gauss_y.t();
}

Do note that if you intend to use the kernel for filtering, it's best to keep it separated, and perform horizontal and vertical passes each with a one-dimensional kernel.


Find foreign key membership including owner in SQL Server Was hoping someone could sanity-check this SQL statement.  I need to find the owner (schema), table, and column on both sides of a foreign key:

SELECT
    s1.name AS FK_schema,
    o1.name AS FK_table,
    c1.name AS FK_column,
    fk.name AS FK_name,
    s2.name AS PK_schema,
    o2.name AS PK_table,
    c2.name AS PK_column,
    pk.name AS PK_name,
    fk.delete_referential_action_desc AS Delete_Action,
    fk.update_referential_action_desc AS Update_Action
FROM sys.objects o1
    INNER JOIN sys.schemas s1
        ON o1.schema_id = s1.schema_id
    INNER JOIN sys.foreign_keys fk
        ON o1.object_id = fk.parent_object_id
    INNER JOIN sys.foreign_key_columns fkc
        ON fk.object_id = fkc.constraint_object_id
    INNER JOIN sys.columns c1
        ON fkc.parent_object_id = c1.object_id
        AND fkc.parent_column_id = c1.column_id
    INNER JOIN sys.columns c2
        ON fkc.referenced_object_id = c2.object_id
        AND fkc.referenced_column_id = c2.column_id
    INNER JOIN sys.objects o2
        ON fk.referenced_object_id = o2.object_id
    INNER JOIN sys.schemas s2
        ON o2.schema_id = s2.schema_id
    INNER JOIN sys.key_constraints pk
        ON fk.referenced_object_id = pk.parent_object_id
        AND fk.key_index_id = pk.unique_index_id
ORDER BY s1.name, o1.name, s2.name, o2.name, fkc.constraint_column_id

The query will return the schema name properly.  However, note that a foreign key may not necessarily reference the primary key; a unique constraint or unique index may also be referenced.  I suggest renaming the PK_Name column to a more generic name like KEY_Name to more accurately describe the column in cases where it's a unique constraint or index instead of primary key.

With a foreign key referencing a unique index, the relationship will be missed entirely by the query because unique indexes are not reflected in sys.key_constraints.  Consider using sys.indexes instead of sys.key_constraints to avoid that problem.

SELECT
    s1.name AS FK_schema,
    o1.name AS FK_table,
    c1.name AS FK_column,
    fk.name AS FK_name,
    s2.name AS PK_schema,
    o2.name AS PK_table,
    c2.name AS PK_column,
    i.name AS KEY_name,
    fk.delete_referential_action_desc AS Delete_Action,
    fk.update_referential_action_desc AS Update_Action
FROM sys.objects o1
    INNER JOIN sys.schemas s1
        ON o1.schema_id = s1.schema_id
    INNER JOIN sys.foreign_keys fk
        ON o1.object_id = fk.parent_object_id
    INNER JOIN sys.foreign_key_columns fkc
        ON fk.object_id = fkc.constraint_object_id
    INNER JOIN sys.columns c1
        ON fkc.parent_object_id = c1.object_id
        AND fkc.parent_column_id = c1.column_id
    INNER JOIN sys.columns c2
        ON fkc.referenced_object_id = c2.object_id
        AND fkc.referenced_column_id = c2.column_id
    INNER JOIN sys.objects o2
        ON fk.referenced_object_id = o2.object_id
    INNER JOIN sys.schemas s2
        ON o2.schema_id = s2.schema_id
    INNER JOIN sys.indexes i
        ON fk.referenced_object_id = i.object_id
        AND fk.key_index_id = i.index_id
ORDER BY s1.name, o1.name, s2.name, o2.name, fkc.constraint_column_id;


Getting data correctly from  tag with beautifulsoup and regex I am scraping an online shop page, trying to get the price mentioned in that page. In the following block the price is mentioned:

&#x20B9;&nbsp;999

I am using Beautiful Soup to get this tag and using a regular expression to get the price:

# -*- coding: utf8 -*-

import re
import requests
from bs4 import BeautifulSoup

hs18_test_urls = ['http://www.homeshop18.com/diary-wimpy-kid-hard-luck/author:jeff-kinney/isbn:9780141350677/books/juvenile-fiction/product:30926027/cid:10065/?it_category=HP&it_action=BO-R01001&it_label=HP-R01001-140121094603-30926027-PR-BO-RM-OT-RT01_BooksFlat40PercentOff-RL02-160120&it_value=0',
            'http://www.homeshop18.com/apple-ipad-air-wi-fi-16gb-space-grey/computers-tablets/tablets/product:31228967/cid:16327/',
            'http://www.homeshop18.com/reebok-men-black-yellow-sandals-j97184/footwear/men/product:30795219/cid:15067/',
            'http://www.homeshop18.com/american-swan-women-shirt-pink/clothing/women/product:31225645/cid:15021/',
            'http://www.homeshop18.com/diva-fashion-art-silk-saree-parrot-green/clothing/women/product:31514557/cid:15011/?it_category=hs18bot&it_action=recentlySoldProducts&it_label=31225645&it_value=1']

hs18_expected_test_results = [u'210', u'35900', u'1499', u'479', u'1199']

def get_homeshop18_product_meta(url):
    reg = ur'^ ₹? (\d+)'
    response = requests.get(url)
    if response.status_code == requests.codes.ok:
        soup = BeautifulSoup(response.text)
        product_name = soup.find('meta', {'property' : 'og:title'})['content']
        product_url = soup.find('meta', {'property' : 'og:url'})['content']
        product_img_url = soup.find('meta', {'property': 'og:image'})['content']
        product_price_tag_element = soup.find('span', {'id': 'hs18Price', 'itemprop': 'price'})
        product_price_match = re.match(reg, product_price_tag_element.text)
        if product_price_match:
            product_price = product_price_match.group(1)
        else:
            product_price = None
        return (product_url, product_name, product_img_url, product_price)
    return None

if __name__ == '__main__':
    assert(map(lambda x: x[3], map(get_homeshop18_product_meta, hs18_test_urls)) == hs18_expected_test_results)

Questions:

With beautiful soup I can get contents within that span tag. Is there any way to just get the content of the outer span and ignoring whatever is within the inner span tag? i.e. it should give me only &nbsp; 210.
If the above is not possible, then is there any further improvements you suggest with regards to re or the code in general?
I am using a unicode character(\U20B9) in my script.  Should I be doing that?  I couldn't get re to match even after sending re.UNICODE flag.

reg = r'^ \U20B9? (\d+)'

product_price_match = re.match(reg, product_price_tag_element.text, re.UNICODE)

For the reason mentioned in #3, I am using -*- coding: utf8 -*- at the start. Is that okay? Is this the pythonic way? or any suggested way?

As it happens, there are positive answers to each of your questions:

with Beautiful Soup you can remove the WebRupee span with replace_with() entirely....

webrupee_element = soup.find('span', {'class': 'WebRupee'})
webrupee_element.replace_with('')

... then, when you get the text value of the product_price_tag_element.text it will not have the symbol.

EDIT: Of course, it would be faster/better to do:

for wr in product_price_tag_element.find('span'):
    wr.replace_with('')

Your regex is not matching the value properly because the &nbsp; character may not be mapping directly to the regular ' ' character in your regex. You should use the 'whitespace' escape-sequence \s in your regex instead of ' ', like reg = ur'^\s*₹?\s*(\d+)\s*'
Unicode string literals in python should be escaped with either the \u or \U escape, but they are different:

  Specific code points can be written using the \u escape sequence, which is followed by four hex digits giving the code point. The \U escape sequence is similar, but expects 8 hex digits, not 4.

in your case you should use lower-case \u
You can remove the utf-8/unicode declaration if you encode the values in the more-pythonic way of unicode escapes.


MySQL JDBC integration into App from tutorial I was following this tutorial for JavaFX and Gradle:

JavaFX with Gradle, Eclipse and, Scene Builder on OpenJDK11

After completion of the tutorial you will have a simple GUI which you input a lower and upper bound and generate random numbers between them. The generated numbers and their bounds are then saved and listed in another view.

After completion of this tutorial I wanted to integrate MySQL as the back end.

The tutorial creator acknowledged at some point that a database like SQLite would be the preferred back end but that it was outside the scope of the tutorial. Luckily it seems he designed the app in such a way that he uses DAO class and I just replaced his code in there with my own JDBC implementation.

package RandomNumber.repository;

import java.util.*;
import java.sql.*;

import RandomNumber.models.RandomNumber;

public class LocalRandomNumberDAO implements RandomNumberDAO {

    public LocalRandomNumberDAO() {

    }

    private List<RandomNumber> numbers = new ArrayList<>();

    private Connection openConn() throws SQLException {
        return DriverManager.getConnection("jdbc:mysql://localhost:3306/numbers?" +
                                                   "user=demo_java&password=1234");
    }

    @Override
    public boolean save(RandomNumber number) {
        boolean res = false;
        try (
            var conn = openConn();
            var stmt = conn.prepareStatement("insert into numbers_tabl values(?,?,?,?,?)");
        ) {
            stmt.setInt(1, 0);
            stmt.setInt(2, number.getNumber());
            stmt.setInt(3, number.getLowerBounds());
            stmt.setInt(4, number.getUpperBounds());
            stmt.setDate(5, java.sql.Date.valueOf(number.getCreatedAt()));
            res = stmt.execute();
        } catch (SQLException e) {
            e.printStackTrace();
        }
        return res;
    }

    @Override
    public void loadNumbers() {
        try (
            var conn = openConn();
            var stmt = conn.createStatement();
        ) {
            String strQuery = "select * from numbers_tabl;";
            var rset = stmt.executeQuery(strQuery);
            numbers.clear();
            while(rset.next()) {
                numbers.add(new RandomNumber(
                        rset.getDate("created").toLocalDate(),
                        rset.getInt("number"),
                        rset.getInt("min"),
                        rset.getInt("max")
                        )
                    );
            }
        } catch (SQLException e) {
            e.printStackTrace();
        }
    }

    @Override
    public List<RandomNumber> getNumbers() {
        return Collections.unmodifiableList(numbers);
    }
}

I am also calling loadNumbers each time the generate button is pressed, so I essentially insert 1 row, and then query all rows in the database.

Questions:
1. Must I always provide a value for the Primary Key to the PreparedStatement in the save method? my table description follows:

mysql> describe numbers_tabl;
+---------+------+------+-----+---------+----------------+
| Field   | Type | Null | Key | Default | Extra          |
+---------+------+------+-----+---------+----------------+
| id      | int  | NO   | PRI | NULL    | auto_increment |
| number  | int  | NO   |     | NULL    |                |
| min     | int  | NO   |     | NULL    |                |
| max     | int  | NO   |     | NULL    |                |
| created | date | YES  |     | NULL    |                |
+---------+------+------+-----+---------+----------------+
5 rows in set (0.00 sec)

2. What best practices am I blatantly ignoring?
3. Is there a better way? Or rather, is there a library or framework that is more worth my time?
4. This is my first time using Java-11. I have always used pretty much only Java 7 or 8 features. Is my usage of the var keyword appropriate?

You're not following Java naming conventions. Package names should be all lower case.

The JDBC URL is hard coded. This should be provided as a parameter. Or better yet, provide the connection with dependency injection so you can unit test it with mocks.

Worst "offense" is hiding the exceptions from the caller. Instead of printing the stack trace (which is only visible to the end user) you should wrap it in a domain specific exception. This way the application using your library can take proper actions.

When saving the number you list all columns as parameters in the SQL. Including the automatically generated index, which you insert as zero. That was confusing. You should list only the columns that you provide as "INSERT INTO numbers_tabl (number, min, max) ..." and let the database handle the columns that can be generated automatically (index and creation date).

Instead of returning a boolean from saveNumber you could return the RandomNumber object updated with it's new database identifier and creation date.

The names in the database table don't describe their purposes. Table us named "numbers_tabl" but it contains random numbers. The name should tell the reader what the table contains. Also the _tabl suffix is probably a bit pointless. The "created" column name suggests that the value is a boolean. It should be "creation_time". I don't remember MySQL data types from memory but it probably should be a timestamp, not just date.

The method choice comes from the tutorial, right? It's a bit weird to load the numbers into memory and access them from there. After saving a number you have to call loadNumbers before you get the latest number back from getNumbers. The getNumbers should load the numbers from the database. If you want to cache the numbers in memory then the saveNumbers should invalidate the cache, but that's probably a subject for a later tutorial. :) Also there are libraries for that (Ehcache for example).

The standard library for this purpose is JPA (Java Persistence API or Jakarta Persistence). It makes boilerplate SQL so much easier.


Coding adjacency list representing graph I want to write some basic code on graph like DFS, BFS but before that I am coding graph representation in adjacency list, and some basic functions like readgraphfrommatrix. (This will initialize graph from adjacency matrix which is easier to supply as input to test the program.)

A vertex can have some info like name (say name of town), an edge can have some info like weight.

Does below code look decent enough or there are more standard recognized code snippets for adjacency list representation of graphs?

struct edge
{
    char *data;
}

struct vertex
{
    char *data;
};

struct node
{
    int vertexnumber;
    struct node *next;
}

struct graph
{
    int nvertex,nedges; //
    struct vertex **V; // in main, I will malloc depending on input to have sth like struct vertex V[nvertex];
    struct edge **E //to have like struct edge E[nedges];
    struct node **list ; // to have like struct node* list[nvertex];
}

Code style: Use typedef struct instead of struct for cleanliness.

This looks generally OK, but is not strictly speaking an adjacency list representation. In an adjacency list representation you store a list of verticies, and each of those verticies store a list of its neighbours. This structure implicitly encodes the edges of the graph.

A more compact adjacency representation might look something more like this:

typedef struct{
    char *data;
    struct vertex **neighbours;
    char **weight; // edge to neighbours[i] stored in weight[i]
} vertex;

typedef struct{
    int num_verticies;
    struct vertex **V;
} graph;

The edges can be recovered by traversing the graph. To improve performance we may choose a structure other than a linked list to store the neighbours of a vertex.

Which further annotations you keep in the graph struct will depend on the final use case. For certain algorithm having an explicit edge list will be vital, whereas for others it will be wasteful.


Bash script to swap out, edit host files This is my first useful bash script. It's meant to make it easy to switch between a "work" hosts file (that blocks sites like reddit.com, youtube.com, etc.) and my normal hosts file, and also to allow me to easily add/remove sites I want to block.

I'm not sure if I'm doing everything right or securely, so please feel free to pick at anything.

#!/bin/bash

usage()
{
cat <<EOF
Usage: wrk [command] [host1 host2 ...]

Commands:
    list                list blocked hosts
    add [host ...]      add a host to be blocked
    rm [host ...]       remove hosts from block
    start               start blocking
    stop                stop blocking
EOF
}

VARDIR="$HOME/abes_commands/var"
# HOST_FILE - actual host file to be swapped around.
# ORIG_FILE - original host file without any of the appended blocked hosts.
# BLCK_FILE - list of blocked hosts.
# BLCK_TEMP - temporary file used when removing hosts.
HOST_FILE="/etc/hosts"
ORIG_FILE="$VARDIR/original_host"
BLCK_FILE="$VARDIR/blocked_host"
BLCK_TEMP=`mktemp -t "blocked_hosts"` || `mktemp /tmp/blocked_hosts.XXXXXXX` || exit 1

# Make sure files exist.
[[ -e $ORIG_FILE ]] || touch "$ORIG_FILE"
[[ -e $BLCK_FILE ]] || touch "$BLCK_FILE"

# Check to see if the block is currently active.
ACTIVE_FLAG="$HOME/.wrk_block.flag"
if [[ -e $ACTIVE_FLAG ]]; then
    IS_ACTIVE=0
else
    IS_ACTIVE=1
fi

add_host()
{
    local hosts=("$@")
    for host in "${hosts[@]:1}"; do
        # append host to blocked hosts list
        echo "127.0.0.1 $host" >> "$BLCK_FILE"
        echo -e "\033[0;32madded\033[0m $host"
    done
}

# todo: remove need for loop; do in one go
rm_host()
{
    local hosts=("$@")
    for host in "${hosts[@]:1}"; do
        # overwrite host list file with a copy removing a certain host
        awk -v host=$host 'NF==2 && $2!=host { print }' "$BLCK_FILE" > "$BLCK_TEMP"
        mv "$BLCK_TEMP" "$BLCK_FILE"
        echo -e "\033[0;31mremoved\033[0m $host"
    done
}

check_root()
{
    if [[ "`whoami`" != "root" ]]; then
        echo "You don't have sufficient priviledges to run this script (try sudo.)"
        exit 1
    else
        [[ -e $HOST_FILE ]] || { echo "Can't find or access host file."; exit 1; }
    fi
}

start_block()
{
    if [[ $IS_ACTIVE -ne 0 ]]; then
        cp "$HOST_FILE" "$ORIG_FILE"
        cat "$BLCK_FILE" >> "$HOST_FILE"
        touch "$ACTIVE_FLAG"
        echo "Block started."
    else
        echo "Already blocking."
    fi
}

stop_block()
{
    if [[ $IS_ACTIVE -eq 0 ]]; then
        cp "$ORIG_FILE" "$HOST_FILE"
        [[ -e $ACTIVE_FLAG ]] && rm "$ACTIVE_FLAG"
        echo "Stopped blocking."
    else
        echo "Not blocking."
    fi
}

case $1 in
    'ls' | 'list')
        awk 'NF == 2 { print $2 }; END { if (!NR) print "Empty" }' "$BLCK_FILE";;
    'add')
        [[ -z $2 ]] && { usage; exit 1; }
        add_host $@;;
    'rm' | 'remove')
        [[ -z $2 ]] && { usage; exit 1; }
        rm_host $@;;
    'start')
        check_root
        start_block;;
    'stop')
        check_root
        stop_block;;
    *)
        usage;;
esac

The blocked_host file is appended to the hosts file when blocking starts, and looks like this:

127.0.0.1 www.reddit.com
127.0.0.1 reddit.com
127.0.0.1 www.news.ycombinator.com
127.0.0.1 news.ycombinator.com

I think this is generally clean.

Style Points

use $() instead of ``, it's clearer and they can be nested.

E.g:

if [[ "$(whoami)" != "root" ]]; then
    echo "You don't have sufficient priviledges to run this script (try sudo.)"
    exit 1

Consider setting:

set -u #prevent unset variables
set -e #stop on an error

At the top of your script.

rm_host may benefit from rewriting using an inplace sed edit, rather than using awk to copy over the top of the old file.

Locks

Although unlikely, you may want to consider a lock to prevent races between add_host and rm_host. $ACTIVE_FLAG would make a good lock.

$VARDIR

It looks like you intend this script to be used at differing privileges level (e.g. root and non-root). This will change the $HOME env variable and consequently the $VARDIR env variable. Potentially {add,rm}_host may act on different files to {start,stop}_block. Note that this won't present itself  if you only use sudo.

Reconsider what you want $VARDIR to be. On a a single user machine /var/ may be a good choice.


Parsing console application arguments In our console app, the parsing of application arguments is done like so:

using System.Linq;

namespace Generator
{
    internal class Program
    {
        public static void Main(string[] args)
        {
            var param1 = args.SingleOrDefault(arg => arg.StartsWith("p1:"));
            if (!string.IsNullOrEmpty(param1))
            {
                param1  = param1.Replace("p1:", "");
            }
            //...
        }
    }
}

It's supposed to be called like this:

`Generator.exe p1:somevalue`

Is there a better/simpler way to parse arguments?

I'd recommend taking advantage of the excellent Mono.Options module.  It's a single .cs file you can drop in to your solution and get full-featured parsing of GNU getopt-style command lines.  (Things like -x -y -z, which is equivalent to -xyz, or -k value, or --long-opt, and so forth.)


Convert decimal to binary and find the maximum number of consecutive 1's in the binary Task: given a base- 10 integer, n, convert it to binary (base-2). Then find and print the base- 10 integer denoting the maximum number of consecutive 1's in n's binary representation.

What can you say about my implementation of the problem?

n = gets.strip.to_i
array = []
while n > 0
  array.push(n % 2)
  n = n / 2
end

counter = 0
consec = []
len = array.length

array.each_with_index do |x, index|
  if x == 1
    counter += 1
    consec.push(counter) if index + 1 == len
  else
    consec.push(counter) if counter > 0
    counter = 0
  end
end

puts consec.sort[-1]

You solution looks good for a procedural one -- something similar would be the correct way to do this in, say, C.

That said, and assuming this isn't considered "cheating" for the problem at hand, if you're using ruby, you might as well take advantage of ruby's facilities.  Assuming the input number is n, all you need to do is:

n.to_s(2).scan(/1+/).max_by(&:size).size

or, even more tersely, taking advantage of how strings are compared (ie, "11" > "1" and "111" > "11", etc):

n.to_s(2).scan(/1+/).max.size

to_s(2) converts to the binary string, scan finds all runs of 1s, max_by finds the longest of those, and the final size tells you how long that is, as a decimal.


Designing DAOs that handle composite objects Suppose I have following elementary DTOs, each DTO corresponding to a database table:

class Event {
    String name
}
class Player {
    String name
}

DAOs for elementary DTOs:

class PlayerDao {
    def insert(Player p) {
        //sql to insert local members
    }
}
class EventDao {
    def insert(Event e) {
        //sql to insert local members
    }
}

Composite DTO related to other DTOs via foreign keys:

class Game {
    Event e
    Player p
    Date d
}

Now there are two approaches to designing the DAO for game object:

Have the caller hand over entire game object to GameDao and GameDao calls other DAOs to save the child objects.

class GameDao {
    def insert(Game g) {
        eventDao.insert(g.e)
        playerDao.insert(g.p)
        //sql to insert local members
    }
}
class Caller {
    def static main(String[] a) {
        def g = new Game()
        gameDao.insert(g)
    }
}

Make GameDao save only local state of object and leave the responsibility of saving related objects to caller.

class GameDao {
    def insert(Game g) {
        //sql to insert local members
    }
}
class Caller {
    def static main(String[] a) {
        def g = new Game()
        eventDao.insert(g.e)
        playerDao.insert(g.p)
        gameDao.insert(g)
    }
}

Use of an ORM is out of question.

Which design is better and why?

I would go for the first one, because it exposes a much simpler API to the caller.

The day you need to add anything to your design, if it's encapsulated this way, the caller doesn't need to change a thing and it's still going to work, because the Game knows how to build itself and doesn't put that burden onto the caller.