Decrypting PDF-Files In Encrypt PDF-Files I provided a Python program to encrypt PDF-Files in a folder.

This program here does the oposite. Giving a password and path all PDFs which are encrypted are decrypted in the supplied path. If the password is wrong a message is displayed.

pdf_decrypter.py

"""
Decrypts all files in a folder and sub folder with provided password
"""

import os
import argparse
from typing import Tuple
from getpass import getpass
from pathlib import Path
import PyPDF2

def get_password_from_user() -> str:
    """Asks the user to enter a password"""
    while True:
        password: str = getpass(promt="Enter password: ")
        if password:
            return password

def get_path_from_user() -> str:
    """Asks for a path from the User"""
    while True:
        #input_string = "E:/Python Projekte\ATBS\CH13_EX1_pdf_decrypter\CH13_EX1_pdf_decrypter\CH13_EX1_pdf_decrypter\\testfolder"
        path = Path(input("Enter absolute Path:"))
        if os.path.exists(path):
            return path
        print("Path doesn't exist\n")

def get_path_and_password() -> Tuple[str, str]:
    """
    Gets path and password from command line or
    if not available from the user
    """
    parser = argparse.ArgumentParser()
    parser.add_argument(
        "--path", help="provide root folder to look for PDFs to encrypt",
        type=str)
    parser.add_argument(
        "--password", help="password to encrypt PDFs with", type=str)
    args = parser.parse_args()
    if args.path and args.password:
        return Path(args.path), args.password
    return get_path_from_user(), get_password_from_user()

def is_encrypted(filename: str) -> bool:
    """Checks if file is encrypted"""
    with open(filename, 'rb') as pdf_file:
        pdf_reader = PyPDF2.PdfFileReader(pdf_file)
        return pdf_reader.isEncrypted

def decrypt(filename: str, password: str) -> bool:
    """
    Try to decrypt a file. If not successful a false is returned.
    If the file passed is not encrypted also a false is passed
    """
    with open(filename, 'rb') as pdf_file:
        pdf_reader = PyPDF2.PdfFileReader(pdf_file)
        pdf_reader.decrypt(password)
        pdf_writer = PyPDF2.PdfFileWriter()

        try:
            for page_number in range(pdf_reader.numPages):
                pdf_writer.addPage(pdf_reader.getPage(page_number))
        except PyPDF2.utils.PdfReadError:
            return False

        filename_decrypted = filename.strip('.pdf') + "_decrypted.pdf"

        with open(filename_decrypted, 'wb') as pdf_file_decrypted:
            pdf_writer.write(pdf_file_decrypted)
    return True

def pdf_decrypter():
    """
    Main routine:
    Giving a password and path all PDFs which are encrypted get
    decrypted in the supplied path.
    If the password is wrong a message is displayed.
    """
    path, password = get_path_and_password()
    for folder_name, _, filenames in os.walk(path):
        for filename in filenames:
            if not filename.endswith('.pdf'):
                continue

            if not is_encrypted(os.path.join(folder_name, filename)):
                continue

            if decrypt(os.path.join(folder_name, filename), password):
                os.remove(filename)
            else:
                print(filename + " could not be decrypted (wrong password)")

if __name__ == "__main__":
    pdf_decrypter()

I already incorporated improvements suggested in the encrypter review in Encrypt PDF-Files

What else can be improved in the code?

Let me know.

Bugs:

Here, instead of promt it should be prompt:

password: str = getpass(promt="Enter password: ")

If PDF file is not in the current folder, this will throw an error:

if decrypt(os.path.join(folder_name, filename), password):
    os.remove(filename)

it should be os.remove(os.path.join(folder_name, filename)) instead.

Docstring:

"""
Decrypts all files in a folder and sub folder with provided password
"""

I think it would be better to specify that only PDF files will be decrypted. Also, it should mention that original copy will be removed. How about:

"""
Decrypts all pdf files in a specified folder
and all its sub-folders with provided password,
saves decrypted copy and removes original encrypted files.
"""

PEP 8:

  Imports should be grouped in the following order:

  Standard library imports.
  Related third party imports.
  Local application/library specific imports.

  You should put a blank line between each group of imports.

Also, it is advised to sort imports alphabetically.

So, instead of:

import os
import argparse
from typing import Tuple
from getpass import getpass
from pathlib import Path
import PyPDF2

You will have:

import argparse
import os
from getpass import getpass
from pathlib import Path
from typing import Tuple

import PyPDF2

Also, before the first function there should be two blank lines. Apart from that, well done! You follow the style guide pretty well!

Click:

Instead of using argparse, I'd suggest to take a look at third-party library Click. Why Click?

Example of usage:
For example, here is how you could wrap main function pdf_decrypter:

import click

...

@click.command()
@click.option('--path', '-p',
              default='.',
              show_default=True,
              type=click.Path(),
              help='Folder path to look for PDFs to decrypt '
                   '(absolute or relative).')
@click.password_option(confirmation_prompt=False)
def pdf_decrypter(path: str,
                  password: str):
...

Running python3 pdf_decrypter.py --help from command line will print:

Usage: pdf_decrypter.py [OPTIONS]

  Main routine: Giving a password and path all PDFs which are encrypted get
  decrypted in the supplied path. If the password is wrong a message is
  displayed.

Options:
  -p, --path PATH  Folder path to look for PDFs to decrypt (absolute or
                   relative). [default: .]
  --password TEXT
  --help           Show this message and exit.

Confirmation messages:
Further in the body of the function you can add confirmation message as well, like:

...
if not click.confirm(f'All PDF files will be encrypted in {path}\n'
                     'Do you want to continue?'):
    return
...

This will look like:

python3 pdf_decrypter.py
Password:
All PDF files will be encrypted in /path/to/pdf_files
Do you want to continue? [y/N]:

Putting encryption and decryption together:
And, probably, you would want to put both encryption from your previous post and decryption in the same module later, as they share a lot in common. In this case you would want to use Nesting Commands, for example, like this:

@click.group()
def cli():
    pass

@cli.command()
@click.option('--path', '-p',
              default='.',
              show_default=True,
              type=click.Path(),
              help='Folder path to look for PDFs to decrypt '
                   '(absolute or relative).')
@click.password_option(confirmation_prompt=False)
def decrypt(path: str,
            password: str):
...

@cli.command()
@click.option('--path', '-p',
              default='.',
              show_default=True,
              type=click.Path(),
              help='Folder path to look for PDFs to decrypt '
                   '(absolute or relative).')
@click.password_option(confirmation_prompt=False)
def encrypt(path: str,
            password: str) -> None:
...

And you would call your functions like python pdf_utils.py encrypt or python pdf_utils.py decrypt, etc.

click.Path:
You can specify additional checks, like checking if path exists, path is not a file, and also make Click to resolve path automatically by setting resolve_path as True. So you wouldn't have to use os.path.join later in the code:

@click.option('--path', '-p',
              default='.',
              show_default=True,
              type=click.Path(exists=True,
                              file_okay=False,
                              resolve_path=True),
              help='Folder path to look for PDFs to decrypt '
                   '(absolute or relative).')

Iterating over files:

Right now you have two nested loops and two checks of extension and if file is encrypted:

for folder_name, _, filenames in os.walk(path):
    for filename in filenames:
        if not filename.endswith('.pdf'):
            continue

        if not is_encrypted(os.path.join(folder_name, filename)):
            continue

        # decryption logic

This looks pretty cumbersome. And I personally don't like it when the code is nested on two or more levels. We can use pathlib module to simplify this logic:

pdf_paths = Path(path).rglob('*.pdf')
encrypted_pdfs = filter(is_encrypted, pdf_paths)
for pdf in encrypted_pdfs:
    # decryption logic

Errors catching:

Now you catch PyPDF2.utils.PdfReadError on the deepest level, in decrypt function. I suggest you to take it out to main function, and not return True or False to indicate success/failure. See, for example, When and how should I use exceptions?
So, in the pdf_decrypter, you will have:

for pdf in encrypted_pdfs:
    try:
        decrypt(pdf, password)
        pdf.unlink()
    except PyPDF2.utils.PdfReadError:
        print(f'{pdf} could not be decrypted (wrong password)')

pathlib over os:

You almost don't use advantages of the pathlib. I already included some things in previous code examples. But here I will sum up everything.

Opening file:
This:

with open(filename, 'rb') as pdf_file:

will become:

with filepath.open('rb') as pdf_file:

Note that filepath would be a better name here.

Changing filename:

filename_decrypted = filename.strip('.pdf') + "_decrypted.pdf"

could be rewritten as:

decrypted_filename = f'{filepath.stem}_decrypted{filepath.suffix}'  # new name
decrypted_filepath = filepath.parent / decrypted_filename  # complete path

Removing file:
os.remove(path) can be replaced by path.unlink().
By this moment we removed all os module usages. So, we can remove that import.

Putting it all together:

"""
Decrypts or encrypts all pdf files in a specified folder
and all its sub-folders with provided password,
saves processed copies and removes original files.
"""
from itertools import filterfalse
from pathlib import Path

import click
import PyPDF2

def is_encrypted(filepath: Path) -> bool:
    """Checks if file is encrypted"""
    with filepath.open('rb') as pdf_file:
        pdf_reader = PyPDF2.PdfFileReader(pdf_file)
        return pdf_reader.isEncrypted

def write_copy(filepath: Path,
               password: str,
               *,
               mode: str) -> None:
    """
    Writes encrypted or decrypted copy of the file based on the mode
    :param filepath: path of the PDF file to be processed
    :param password: password to decrypt/encrypt PDF with
    :param mode: one of 'encrypt' or 'decrypt'
    """
    with filepath.open('rb') as pdf_file:
        pdf_reader = PyPDF2.PdfFileReader(pdf_file)
        pdf_writer = PyPDF2.PdfFileWriter()

        if mode == 'decrypt':
            pdf_reader.decrypt(password)
        for page_number in range(pdf_reader.numPages):
            pdf_writer.addPage(pdf_reader.getPage(page_number))
        if mode == 'encrypt':
            pdf_writer.encrypt(password)

        suffix = {'decrypt': '_decrypted',
                  'encrypt': '_encrypted'}
        processed_filename = f'{filepath.stem}{suffix[mode]}{filepath.suffix}'
        processed_filepath = filepath.parent / processed_filename

        with processed_filepath.open('wb') as pdf_file_processed:
            pdf_writer.write(pdf_file_processed)

@click.group()
def cli():
    pass

@cli.command()
@click.option('--path', '-p',
              default='.',
              show_default=True,
              type=click.Path(exists=True,
                              file_okay=False,
                              resolve_path=True),
              help='Folder path to look for PDFs to decrypt '
                   '(absolute or relative).')
@click.password_option(confirmation_prompt=False)
def decrypt(path: str,
            password: str) -> None:
    """
    Decrypts all encrypted PDFs in the supplied path.
    If the password is wrong a message is displayed.
    Saves processed copies and removes original files.
    :param path: folder path to look for PDFs to decrypt
    :param password: password to decrypt PDFs with
    """
    pdf_paths = Path(path).rglob('*.pdf')
    encrypted_pdfs = filter(is_encrypted, pdf_paths)
    for pdf in encrypted_pdfs:
        try:
            write_copy(pdf, password, mode='decrypt')
            pdf.unlink()
            click.echo(f'Decrypted successfully: {pdf}')
        except PyPDF2.utils.PdfReadError:
            click.echo(f'{pdf} could not be decrypted (wrong password)')

@cli.command()
@click.option('--path', '-p',
              default='.',
              show_default=True,
              type=click.Path(exists=True,
                              file_okay=False,
                              resolve_path=True),
              help='Folder path to look for PDFs to decrypt '
                   '(absolute or relative).')
@click.password_option(confirmation_prompt=False)
def encrypt(path: str,
            password: str) -> None:
    """
    Encrypts all non-encrypted PDFs in the supplied path.
    Saves processed copies and removes original files.
    :param path: folder path to look for PDFs to encrypt
    :param password: password to encrypt PDFs with
    """
    if not click.confirm(f'All PDF files will be encrypted in {path}\n'
                         'Do you want to continue?'):
        return

    pdf_paths = Path(path).rglob('*.pdf')
    not_encrypted_pdfs = filterfalse(is_encrypted, pdf_paths)

    for pdf in not_encrypted_pdfs:
        write_copy(pdf, password, mode='encrypt')
        pdf.unlink()
        click.echo(f'Encrypted successfully: {pdf}')

if __name__ == '__main__':
    cli()

Probably, encrypt and decrypt could be put in one function with a mode parameter, like what I did in a write_copy function; but see for yourself if it will make the code more clear.


Better way for accumulating a value than using inject/reduce with conditional operator? When there is a situation where I want to accumulate a value I see myself using inject/reduce with a conditional statement. E.g., when I want to add 1 to a value each time an element in a list has a certain value I do something like this:

puts names.reduce(0) {|acc, name| triangles.include?(name.points) ? acc+=1 : acc}

Here names is an array of names, triangles is a list of the first 30 triangle integers (1/2n(n+1)) and points is a method added to class String which calculates the integers value of each character in the name and adds them. So the point is to check how many names in the list are triangle values when you calculate the points in that way.

The issue I have is with the conditional area ([conditional] ? acc+=1 : acc). You can also rewrite the method as:

result = 0
names.each {|name| result +=1 if triangles.include?(name.points)}
puts result

But this method uses three lines and has to initialize the variable at first.

Is there a better way to keep track of the amount (e.g., result, or the acc in the inject/reduce) that does not use the conditional operator (?) or costs three lines of code?

There's a built-in count method which handles your example cleanly:

names.count {|name| triangles.include?(name.points) }

Also, the accumulator version should use +, not +=. reduce uses the block's return value; there's no need to modify acc.

In general, though, there's no simpler way to accumulate values than reduce. It's not very clear, but all of the alternatives are worse.

Don't be afraid of the conditional operator. It's just a different way to spell if...else.


Convert a dictionary of iterables into an iterator of dictionaries I have a user-editable dictionary of content used to fill in a string.Template and I wanted to allow the user to fill the same template with several values at once.

Using only values, itertools.product would be enough for my needs but I needed dictionaries to fill in a template. I also wanted users to be able to enter a single string or numerical value as values for the dictionary without having anything blowing up and requiring them to encapsulate their single value in a 1-length tuple.

I used itertools.product(*map(pair_iter_safe, <the_dict>.items())) to generate iterables of key-value pairs suitable for the dict() constructor or the dict.update() method. With pair_iter_safe being:

def pair_iter_safe(kv_pair):
    key, value = kv_pair
    if isinstance(value, str):
        value = (value,)
    else:
        try:
            value = iter(value)
        except TypeError:
            value = (value,)

    for val in value:
        yield key, val

For a broader picture here is a simplified example of calling code:

import sys
import string
import itertools

PARAMETERS = {
    'name': 'John Doe',
    'age': range(40,45),
    'activity': ('foo', 'bar', 'baz'),
}

SCENARIO = """\
[@campaign] @name:
    Hi! I'm @age and I like to @activity.
"""

class AtTemplate(string.Template):
    delimiter = '@'

def run_scenario(parameters, stdout):
    scenario = AtTemplate(SCENARIO).substitute(parameters)
    print(scenario, file=stdout)

def run_campaign(campaign_name='test', stdout=sys.stdout):
    parameters = {'campaign': campaign_name}
    for setup in itertools.product(*map(pair_iter_safe, PARAMETERS.items())):
        parameters.update(setup)
        run_scenario(parameters, stdout)

def pair_iter_safe(kv_pair):
    key, value = kv_pair
    if isinstance(value, str):
        value = (value,)
    else:
        try:
            value = iter(value)
        except TypeError:
            value = (value,)

    for val in value:
        yield key, val

if __name__ == '__main__':
    run_campaign()

Is there anything wrong with the approach or the pair_iter_safe function altogether?

Interesting question.

I decided to look only at itertools.product(*map(pair_iter_safe, <the_dict>.items())) and pair_iter_safe.

The way I look at it is from a theoretic memory usage perspective. Due to how you have defined it, all (key, value) pairs need to be in memory at the same time, which could take a bit of memory.

I would suggest the following:

def iter_safe(value):
    if isinstance(value, str):
        value = (value,)
    try:
        iter(value)
    except TypeError:
        return (value,)
    else:
        return value

And using it as follows:

def dict_combinations(d):
    keys, values_list = zip(*d.items())
    for values in itertools.product(*map(iter_safe, values_list)):
        yield dict(zip(keys, values))

To me this is cleaner, having separated out the iter_safe logic from the dicts.

Though merely theoretical, I think this should save on memory usage due to there not being as much key-value pairs in memory all the time.

Of course, if you still want pair_iter_safe, you should be able to do something like

def pair_iter_safe(kv_pair):
    key, value = kv_pair
    for v in iter_safe(value):
        yield key, v


Armstrong Number Validator Purpose

Validate if a number is an Armstrong Number.

An Armstrong Number is a number

  such that the sum of the cubes of its digits is equal to the number itself. For example, 371 is an Armstrong number since 33 + 73 + 13 = 371.

Strategy

Calculate the remainder and quotient of the candidate value.
While the quotient is greater than 0 and the digit sum is not greater than the candidate value, calculate the new remainder, the new quotient, and add remainder raised by 3 to the digit sum.

Implementation

public class ArmstrongNumberValidatorImpl implements ArmstrongNumberValidator {
    @Override
    public boolean isArmstrong(final long candidate) {
        if (candidate < 0) {
            throw new IllegalArgumentException("candidate value must be non-negative");
        }

        long remainder = candidate % 10;
        long quotient = candidate / 10;
        long digitSum = Math.multiplyExact(Math.multiplyExact(remainder, remainder), remainder);
        while (quotient > 0 && digitSum <= candidate) {
            remainder = quotient % 10;
            quotient = quotient / 10;
            digitSum += Math.multiplyExact(Math.multiplyExact(remainder, remainder), remainder);
        }

        return (0 == quotient && digitSum == candidate);

    }
}

Why use multiplyexact?
I'm puzzled why you wrote this:

digitSum += Math.multiplyExact(Math.multiplyExact(remainder, remainder), remainder);

instead of this:
digitSum += remainder * remainder * remainder;

You already know that remainder is a single digit value so computing the cube of it will never overflow.
Extraneous check
Your final return statement could be reduced from this:

return (0 == quotient && digitSum == candidate);

to this:
return (digitSum == candidate);

Your while loop will only terminate on this condition:
quotient == 0 || digitSum > candidate

which means that when digitSum == candidate, quotient must always be 0.


Better way to work with clickable regions of window The Goal
Basically, what I'm looking for is the ability to click a region below an image to trigger a click() event which will scroll the window to the following image.
The Question
Is there a better way to do this? I've achieved the result I want, but with a whole ton of <divs>? I know there are some plugins out there but I'd like to do it without including a large library.
The progress...
I've done this by placing several "invisible"
 elements around the image which are bound to click events. Later on I'll try to work through left, right, and top scrolling.
A live preview is available here.
Here is the code:
...

  // these are the "invisible divs"

$("document").ready(function() {

    $(".bottomclick").click(function() {
        console.log("Scroll jquery is working");
        $( this ).parent().addClass("activeproj");
        let $target = $(".activeproj").next(".main-proj");
        if ($target.length == 0 )
            $target = $(".main-proj:first");

        $([document.documentElement, document.body]).animate({
        scrollTop: $($target).offset().top
        }, 2000);

        $('.activeproj').removeClass('activeproj');
    });
});

I think the overall approach of having lots of
s to act as clickable areas is just fine. An alternative is to dynamically calculate the position of a click on main-proj and see if it's sufficiently in one of the directions to count as an intent to scroll, but that's more complicated and more prone to breakage than having separate divs.
Don't be afraid of WET HTML markup in the rendered output. It's perfectly normal and isn't inelegant or indicative of a problem. It's true that having huge numbers of DOM nodes on a page can slow things down, but 4 extra
s for each image isn't incredibly likely to be a tipping point for that - and if it does turn out to be one, consider changing the page so that only a reasonable number of .main-projs are rendered at a time, like 50 or 100, but not much more than that.
That said, it would be good to avoid WET HTML in your source code if possible, if feasible given your setup - you don't want to copy-paste those 7 lines of

    ...

over and over again every time you want to add an image. It'll make understanding the code at a glance harder, and can make refactoring more difficult. To only write the template for such a section once, you can either do it server-side with a template engine, or client-side with something like React, or even with vanilla DOM methods like document.createElement.
Another thing to consider - if the different clickable divs result in different functionality depending on which was clicked, that would be a great thing to indicate to the user. For example, you might want to change the color of a hovered div, perhaps with an arrow or something, to give the user a visual indicator of what will happen when clicked. This is trivial to achieve with separate
s, but annoyingly complicated without it. (If you don't give the user a visual indicator of the difference between clicks on different sections of .main-proj, it's probably not as user-friendly as it should be.)
Other suggestions regarding your code:
Use const, avoid let - see here. It's good to indicate to future readers of the code (often including yourself) that a variable will not be reassigned.
Don't re-select elements you already have a reference to - you don't need to select the .activeproj again, select the parent again instead, or save it in a variable:
const $thisProj = $(this).parent();
// do you want to remove .activeproj from other elements at this point?
// or, if activeproj isn't used in the CSS, you could remove it completely
$thisProj.addClass('.activeproj');
const $target = $thisProj.next('.main-proj');
// ...

Unless you have other kinds of children in this projimgarray (I think you shouldn't), you can also omit the parameter to .next.
No need to wrap a jQuery object in jQuery again This:
scrollTop: $($target).offset().top

can be
scrollTop: $target.offset().top

projimgarray? Arrays are a JavaScript construct. DOM elmements aren't really arrays. Perhaps call it something more precise, and use dashed-case per convention (and to make the word separations easier to understand at a glance) - maybe something like all-projects.


Improving performance in a Webdriver method I'm testing an application which is not exactly an e-commerce application but which behaves enough like one that you can think of it as an e-commerce application for the purposes of understanding this code :)

This code relies heavily on the Selenium Webdriver library.

The code comes from the Product page, where you have searched and found a list of products. This is a utility method to get the list of products being displayed so I can verify correctness along any axis. This method only gets those products that are visible, which may differ from the total list of products if filtering is involved.

I maintain a list of WebElements representing the container for the product display, one for each product on the page, which is initialized in initLists() (along with some other non-important lists) so that we don't have stale references. That's what's stored in allProducts.

ProductView is an inner class that encapsulates a specific Product record in the list; interactions with a given product (such as clicking any of the links for more information) are delegated to the ProductView for that Product.

public ArrayList<Product> getAllVisibleProducts() {
    initLists(); //Reinitialize lists

    Utils.WaitForAjax(driver);
    ArrayList<Product> retval = new ArrayList<Product>();
    for (WebElement rProduct : allProducts) {
        try {
            if (rProduct.isDisplayed()) retval.add(new ProductView(rProduct).getProduct());
        } catch (StaleElementReferenceException e) {
            continue; //We just ignore that element
        }
    }
    return retval;
}

I'm getting hangs of sometimes multiple minutes where the test appears to do nothing on the product page. I'm currently still trying to reproduce the super-long lag, but at least part of my problem is that this method takes quite a long time to return. I put log statements at various portions of the app, and between starting the loop and returning, it was something like 44 seconds for 8 products. Am I doing too much web scraping here?

(ProductView stores references to the elements that are interactable for the given product and attempts to build an in-memory representation of that product by scraping the page using Webdriver. The code to build the product is the vast majority of the constructor and clocks in about 70-odd lines, so I omitted it).

Ideally I think I need to hear from someone who has used WebDriver before, since this probably boils down to WebDriver best practices.

Bunch of small things not related to your performance problem :

Utils.WaitForAjax should be Utils.waitForAjax. Method in Java start with a lower letter.
Don't return ArrayList, it's an implementation. You should use the interface List. The day you want to change to an LinkedList, it will be easier.
I recommend to always use {} for if, even if you only have just one line. But that is up to you.
I like to name ignoredException an exception that I'm ignoring. Make it more clear.

By looking at your code, I can only guess that your performance bottleneck probably is new ProductView(rProduct) or rProduct.isDisplayed(). If in ProductView your scrapping the page, it could take a lot of time depending on the driver and the method you're doing it.

But with the code your showing us right now, there is nothing particularly wrong.


map + foldl = mapfoldl I was wondering... In the standard library of some Prolog systems, there are meta-predicates like maplist and foldl.

In general, when using meta-predicates (like the ones above) with dcg we always need the "foldl" variants. What about the following mapfoldl/5 (mapfoldl//3)?

mapfoldl(P_4,Xs,Zs,S0,S) :-
   list_mapfoldl_(Xs,Zs,S0,S,P_4).

list_mapfoldl_([],[],S,S,_).
list_mapfoldl_([X|Xs],[Y|Ys],S0,S,P_4) :-
   call(P_4,X,Y,S0,S1),
   list_mapfoldl_(Xs,Ys,S1,S,P_4).

What's your take on this code? They isn't this (or something very similar) already in the stdlibs?

In the Logtalk library, you have a map_reduce/5 predicate:

:- public(map_reduce/5).
:- meta_predicate(map_reduce(2, 3, *, *, *)).
:- mode(map_reduce(+callable, +callable, +term, ?list, ?term), zero_or_more).
:- info(map/5, [
    comment is 'Map a list and apply a fold left (reduce) to the resulting list.',
    argnames is ['Map', 'Reduce', 'Accumulator', 'List', 'Result']
]).

map_reduce(Map, Reduce, Acc, List, Result) :-
    map_reduce_(List, Map, Reduce, Acc, Result).

:- meta_predicate(map_reduce_(*, 2, 3, *, *)).
map_reduce_([], _, _, Result, Result).
map_reduce_([Arg| Args], Map, Reduce, Acc, Result) :-
    call(Map, Arg, Arg2),
    call(Reduce, Acc, Arg2, Acc2),
    map_reduce_(Args, Map, Reduce, Acc2, Result).

A main difference to your code is that you assume a single closure combining both the map and reduce operations in the Logtalk version. You also return the map result in addition to the final fold result.


Closeable to AutoCloseable converter utility for 3rd party java libs I made this utility class to convert Closeable instances to AutoCloseable, so that I can make use of the try-with-resource exception handling mechanism of Java7. This is mainly useful for code which can not be altered.

public class AutoClose<T extends Closeable> implements AutoCloseable, Closeable {

    public final T instance;

    private AutoClose(T instance) {
        this.instance = instance;
    }

    public static <T extends Closeable> AutoClose<T> of(T instance) {
        return new AutoClose<>(instance);
    }

    @Override
    public void close() throws IOException {
        instance.close();
    }
}

usage can be like :

try (AutoClose<SomeType> auto = AutoClose.of(someCloseable)) {
    auto.instance.someWork();
}

feedback much appreciated.

I'll try to say this the best way I can...

Completely useless unnecessary!

Sorry. I couldn't find any better way to say it. The code that you have is well-written, but it is not necessary in the first place.

The reason is that the interface Closeable already lists AutoCloseable as a "superinterface", which means that Closeable extends AutoCloseable. So all Closeable objects are already AutoCloseable.

Which means that your code:

try (AutoClose<SomeType> auto = AutoClose.of(someCloseable)) {
    auto.instance.someWork();
}

Can instead be simply:

try (SomeType auto = someCloseable) {
    auto.someWork();
}

There is no need for your AutoClose wrapper.


Generating double linear sequence in Haskell Need some comments on making the below code more effective. I'm pretty new to Haskell.

The sequence in generateSeq is generated by the rule that if x is in the sequence then x*2+1 and x*3+1 are also in the sequence.

getNumberAt gets the n'th number in the (sorted) generated sequence.

The code works but somehow I feel that I'm overcomplicating this. Any pointers are appreciated!

    generateSeq :: Int -> [Integer]
    generateSeq x
        | x == 0 = [1]
        | otherwise = sort $ prev++toAppend
                         where prev = generateSeq(x-1)
                               toAppend = [next*2+1,next*3+1]
                               next = head $ drop(x-1) prev

    getNumberAt :: Int -> Integer
    getNumberAt n = nub(generateSeq n)!!n

    -- Test this!
    main =
       print $ getNumberAt 321

Repeatedly sorting is asympotically slower than keeping a sorted data structure, nub takes quadratic time, head . drop (x-1) = (!!x), and try to use library-defined recursion combinators instead of explicitly using recursion.

import qualified Data.Set as S

generateSeq :: [Integer]
generateSeq = unfoldr (fmap foo . S.minView) $ singleton 1 where
  foo :: (Integer, S.Set Integer) -> (Integer, S.Set Integer)
  foo (x, s) = (x, S.insert (2*x+1) $ S.insert (3*x+1) s)

looks up comonad stuff on a hunch

import qualified Data.Set as S
import Control.Comonad

generateSeq :: [Integer]
generateSeq = unfoldr ((fmap . extend . uncurry) foo . S.minView) $ singleton 1 where
  foo :: Integer -> S.Set Integer -> S.Set Integer
  foo x = S.insert (2*x+1) . S.insert (3*x+1)


Hangman game, with the number of guesses depending on the word length This is an interactive game between the computer and the player, where the computer chooses a word and the user has to guess the letters in that word.

The computer provides certain number of guesses to the user.

Below is my code:

import random
import string

def load_words():
    print "Loading word list from file..."
    inFile = open("words.txt", 'r', 0)
    line = inFile.readline()
    wordlist = line.split()
    print "  ", len(wordlist), "words loaded."
    return wordlist

wordlist = load_words()

def choose_word(wordlist):
    return random.choice(wordlist)

def avail_letters(guesses,guess):
    return guesses.replace(guess,'')

def fill_blank(random_word,blank,guess):
    blank_new=''
    for i,l in enumerate(random_word):
        if l==guess:
            blank_new+=guess
        else:
            blank_new+=blank[i]
        #print blank_new
    return blank_new

def hangman():
    random_word=choose_word(wordlist)
##    print random_word
    len_word=len(random_word)
    print "I am thinking of a word that is ",len_word," letters long."
    print "-------------"
    no_of_guesses=len_word*2
    guesses=string.ascii_lowercase
    guess=''
    blank="_"*len_word
    while no_of_guesses:
        print "You have ",no_of_guesses," guesses left."
        guesses=avail_letters(guesses,guess)
        print "Available letters: ",guesses
        guess=raw_input("Please guess a letter: ")
        if guess in random_word and guesses:
            blank=fill_blank(random_word,blank,guess)
            print "Good guess: ",blank
        else:
            print "Oops! That letter is not in my word: ",blank
        no_of_guesses-=1
        print "-------------"
        if blank==random_word:
            print "Congratulations, you won!"
            break;
    else:
        print "Sorry, you lose!"

dropping you some small comments about the code you posted

The idea of splitting the code into smaller functions works, the first methods you have are 10 lines or less and is not hard to follow what they do. Would be interesting to go ahead and split some parts of hangman() into smaller pieces as well
The name of the variable for your application main loop can be a bit misleading, would suggest something different instead of no_of_guesses, you could think of a can_guess(counter) boolean function that checks a counter and returns if the counter reached zero
Another part that may be tricky to understand is if blank==random_word, could be easier to read if it was more verbose about what happens, like in if not has_more_characters_to_guess(random_word)
As a general advice, don't be shy to have longer function names. available_letters is preferred to any shorter name. fill_blank can be replaced with get_partial_word_guess which may be more legible and explain a bit more on the insights


For Loop with Index to Functional Trying to simply my code below:

for (index, point) in enumerate(self.points){
        let pointButton = UIButton()
        pointButton.frame = point.bounds
        self.view.addSubview(pointButton)
    }

Is there a better way to accomplish this through .map .filter or .reduce ?

The index is not used at all, therefore you do not need enumerate():

for point in self.points { ... }

(You mentioned in a comment that the index is used somewhere else,
but we can review only the code that is shown in your question.)

let pointButton = UIButton()
pointButton.frame = point.bounds

can be simplified to

let pointButton = UIButton(frame: point.bounds)

and then you could get rid of the intermediate variable:

for point in self.points {
    self.view.addSubview(UIButton(frame: point.bounds))
}

and this looks like the best solution in your case to me.

You could use map() for that purpose:

self.points.map {
    self.view.addSubview(UIButton(frame: $0.bounds))
}

But using map() for its side effects is frowned-upon, see for example
Higher order function: “Cannot invoke 'map' with an argument list of type '((_) -> _)'”,
where an explicit foreach method is offered as an alternative:

extension Array {
    func foreach(function: T -> ()) {
        for elem in self {
            function(elem)
        }
    }
}

which can be used as

self.points.foreach {
    self.view.addSubview(UIButton(frame: $0.bounds))
}


Filtering with multiple inclusion and exclusion patterns I have a requirement to be able to filter a list of strings by both inclusion and exclusion patterns (using fnmatch-style wildcards), of which there can be many.

For example, given the set of values:

['a1', 'a2', 'a3', 'b1', 'b2', 'b3', 'c1', 'c2', 'c3']

And these inclusion filters:

['a*', 'b*']

It should return:

['a1', 'a2', 'a3', 'b1', 'b2', 'b3']

It should also support using exclusion filters, e.g. ['c*'] to get the same result.

Lastly, using both inclusion and exclusion filters, the exclusion filters should take precedence in case of a conflict.

Here is my code and tests:

extrafilters.py

import fnmatch

def superFilter(names, inclusion_patterns=[], exclusion_patterns=[]):
    """Enhanced version of fnmatch.filter() that accepts multiple inclusion and exclusion patterns.
    If only inclusion_patterns is specified, only the names which match one or more patterns are returned.
    If only exclusion_patterns is specified, only the names which do not match any pattern are returned.
    If both are specified, the exclusion patterns take precedence.
    If neither is specified, the input is returned as-is."""
    included = multiFilter(names, inclusion_patterns) if inclusion_patterns else names
    excluded = multiFilter(names, exclusion_patterns) if exclusion_patterns else []
    return set(included) - set(excluded)

def multiFilter(names, patterns):
    """Generator function which yields the names that match one or more of the patterns."""
    for name in names:
        for pattern in patterns:
            if fnmatch.fnmatch(name, pattern):
                yield name

extrafilters_test.py

import unittest
from extrafilters import superFilter, multiFilter

class multiFilterTests(unittest.TestCase):
    def setUp(self):
        self.names = ['a1', 'a2', 'a3', 'b1', 'b2', 'b3', 'c1', 'c2', 'c3']

    def test_patterns(self):
        patterns = ['a*', 'b*']
        expected = ['a1', 'a2', 'a3', 'b1', 'b2', 'b3']
        self.assertItemsEqual(list(multiFilter(self.names, patterns)), expected,
            'should include only the names which match one or more of the patterns')

class superFilterTests(unittest.TestCase):
    def setUp(self):
        self.names = ['a1', 'a2', 'a3', 'b1', 'b2', 'b3', 'c1', 'c2', 'c3']

    def test_no_patterns(self):
        expected = self.names
        self.assertItemsEqual(superFilter(self.names), expected,
            'should be the same as the input')

    def test_inclusion(self):
        include = ['a*', 'b*']
        expected = ['a1', 'a2', 'a3', 'b1', 'b2', 'b3']
        self.assertItemsEqual(superFilter(self.names, include), expected,
            'should include only the inclusions')

    def test_exclusion(self):
        exclude = ['c*']
        expected = ['a1', 'a2', 'a3', 'b1', 'b2', 'b3']
        self.assertItemsEqual(superFilter(self.names, exclusion_patterns=exclude), expected,
            'should include all but the exclusions')

    def test_inclusions_and_exclusions(self):
        include = ['a*', 'b*']
        exclude = ['b*']
        expected = ['a1', 'a2', 'a3']
        self.assertItemsEqual(superFilter(self.names, include, exclude), expected,
            'should include the inclusions minus the exclusions')

if __name__ == "__main__":
    unittest.main()

What corner cases will break this, and how is the clarity of the code (function names, docstrings, etc.)? Are the unit tests complete and detailed enough or perhaps too much? Any other style or quality tips are also appreciated.

You can use fnmatch.filter to simplify multiFilter:

for pattern in patterns:
    for name in fnmatch.filter(names, pattern):
        yield name

This still allows an element to be yielded several times, so instead you might want to do:

for name in names:
    if any(fnmatch.fnmatch(name, pattern) for pattern in patterns):
        yield name

You can also avoid creating a second set by using .difference:

return set(included).difference(excluded)

I would probably stick with the original, though. I only mention it because a fair number of people don't know about it and sometimes it does matter.

Your naming breaks PEP8. Don't let unittest fool you; unittest was written by heathens before the style guide was standardized.

A quick touch-up is

import fnmatch

def super_filter(names, inclusion_patterns=[], exclusion_patterns=[]):
    """
    Enhanced version of fnmatch.filter() that accepts multiple inclusion and exclusion patterns.

    If only inclusion_patterns is specified, only the names which match one or more patterns are returned.
    If only exclusion_patterns is specified, only the names which do not match any pattern are returned.
    If both are specified, the exclusion patterns take precedence.
    If neither is specified, the input is returned as-is.
    """
    included = multi_filter(names, inclusion_patterns) if inclusion_patterns else names
    excluded = multi_filter(names, exclusion_patterns) if exclusion_patterns else []
    return set(included) - set(excluded)

def multi_filter(names, patterns):
    """Generator function which yields the names that match one or more of the patterns."""
    for name in names:
        if any(fnmatch.fnmatch(name, pattern) for pattern in patterns):
            yield name

I would also consider changing the description; it's obvious from introspection that inclusion_patterns and exclusion_patterns default to empty, which is counter-intuitive. I would change it to

def super_filter(names, inclusion_patterns=('*',), exclusion_patterns=()):
    """
    Enhanced version of fnmatch.filter() that accepts multiple inclusion and exclusion patterns.

    Filter the input names by choosing only those that are matched by
    some pattern in inclusion_patterns _and_ not by any in exclusion_patterns.
    """
    included = multi_filter(names, inclusion_patterns)
    excluded = multi_filter(names, exclusion_patterns)
    return set(included) - set(excluded)

Your tests cover the general case fine but they don't check edge-cases; what happens with 0-length inputs? What about complicated patterns? Do you ever check precedence, despite having mentioned it?


Web scraper for job listings Is there any room for improvement on this code?

I use mechanize to get the links of a job listing web site. There are pages with pagination (when jobs > 25) and pages without.

If there is, then the link number 5 is labelled after "Next >" and link number 6 is "Last >>".
In order to identify them, I make assertions on their name and act accordingly.
I then check how many pages the pagination occurs, and then, taking the numbers, I scrape the pages one by one.

  def scrape_duty_station
    agent = Mechanize.new
    page=agent.get(self.web_link)

    if page.links[5].text =~ /^next/i
      pagestart=page.links[5].href.match(/\d+/).to_s.to_i
      pageend=page.links[6].href.match(/\d+/).to_s.to_i
      @page=page.links[7...31]
      scrape_single_page
      (pagestart..pageend).each do |i|
        @page=agent.get(self.web_link+"/#{i}")
        @page=@page.links[7...31]
        scrape_single_page
      end
    else
      @page=page.links[5...30]
      scrape_single_page
    end
  end

  def scrape_single_page
    @page.each do |i|
      p=Position.new
      p.title = i.text
      p.web_link = i.href
      p.job_id = i.href.match(/\d+/).to_s
      p.station = Station.where(:web_link => self.web_link).first
      p.save
    end
  end

Here are some comments.
- If you are only @page as a way to communicate with scarpe_single_page, why not make it a parameter?

def scrape_duty_station
    agent = Mechanize.new
    page = agent.get(web_link)

Avoid extra indent when one of the cases is simpler and can be dealt with easily.

Also, about your ranges. They should really be named constants.

    return scrape_single_page(page.links[5...30]) unless page.links[5].text =~ /^next/i
    arr = [page] + (l_i(page,5)..l_i(page,6)).collect{|i|agent.get(web_link+"/#{i}")}
    arr.each {|p| scrape_single_page p.links[7...31] }
  end

If you are using any statement more than once, it may be worthwhile to facter it out to a function.

  def l_i(page, link)
    page.links[link].href.match(/\d+/).to_s.to_i
  end

Does position have a constructor? If not, it would be nice to create one for it. If yes, it would be better to use it.

  def scrape_single_page(pages)
    pages.each do |p|
      Position.new(p.text,p.href,p.href.match(/\d+/).to_s,
            Station.where(:web_link => self.web_link).first).save
    end
  end


Move-aware noexcept stack in C++ I'm learning C++, and move semantics and exception safety seem to be a top priority when using the language. I implemented the simplest data structure I could think of.

#ifndef STACK_CPP11_HPP
#define STACK_CPP11_HPP

#include <vector>

namespace stack {

template <typename T>
class Stack {
private:
  std::vector<T> storage;

public:
  Stack() : storage() {}

  Stack(const Stack<T> &other) {
    *this = other;
  }

  Stack(Stack<T> &&other) {
    *this = other;
  }

  ~Stack() {
  }

  Stack<T> &operator=(const Stack<T> &rhs) {
    if (this != &rhs)
      storage = rhs.storage;

    return *this;
  }

  Stack<T> &operator=(Stack<T> &&rhs) {
    if (this != &rhs)
      storage = rhs.storage;

    return *this;
  }

  bool top(T &ret) const noexcept {
    if (storage.empty())
      return false;

    ret = storage.back();
    return true;
  }

  void push(T item) noexcept {
    storage.push_back(std::move(item));
  }

  bool pop() noexcept {
    if (storage.empty())
      return false;

    storage.pop_back();
    return true;
  }
};

} // namespace stack

#endif // STACK_CPP11_HPP

I tested the code, and it uses the move constructor wherever possible.
Here are the tests:

#include <cassert>
#include <string>

#include "stack.hpp"

using namespace stack;

struct Box {
  std::string label;

  Box(std::string l) : label(l) {}

  friend void swap(Box &lhs, Box &rhs) {
    using std::swap;
    swap(lhs.label, rhs.label);
  }

  Box(const Box &other) {
    label = other.label;
  }

  Box(Box &&other) {
    swap(*this, other);
  }

  ~Box() {
  }

  Box &operator=(Box rhs) {
    swap(*this, rhs);
    return *this;
  }
};

int main(void) {
  Stack<int> stack1;

  int ret;
  assert(!stack1.top(ret));

  stack1.push(100);
  assert(stack1.top(ret));
  assert(stack1.pop());
  assert(ret == 100);

  auto stack2 = Stack<Box>();

  Box box("bar");
  assert(!stack2.top(box));

  stack2.push(std::move(Box{"foo"}));
  assert(stack2.top(box));
  assert(stack2.pop());
  assert(box.label == "foo");

  return 0;
}

Is there a way to improve the code with regards to move semantics and having an overall noexcept behavior? Also, since I'm fairly new to C++, is there anything I could improve in general?

Move Semantics

One of the rules you have to remember is:
A named variable will not bind to an r-value ref parameter.

void doMoreStuff(Type const& x) {std::cerr << "Normal Reference\n";}
void doMoreStuff(Type&& x)      {std::cerr << "RValue Reference\n";}
void doStuff(Type&& x)
{
    doMoreStuff(x); // Here you are using a named variable
}                   // So this will not bind to an r-value ref.
                    // So it will call the top version above.
                    // The `&&` just mark the bind point they
                    // don't change the type of the variable in
                    // any special way.

So your Move Constructor does not do what you think it is doing.

  Stack(Stack<T> &&other) {
    *this = other;  // You are using a named variable.
  }                 // So this will bind to the "Copy Assignment"

To make it work the way you think, you need to use std::move:

  Stack(Stack<T> &&other) {
    *this = std::move(other);
  }

Copy And Swap Idiom

You are implementing the Constructors in terms of Assignment. I suppose it's not wrong. But it is very non-standard. Usually it is done the other way around - implement Assignment in terms of the Constructor (it even has a name: The Copy and Swap Idiom).

  Type& operator=(Type const& rhs)
  {
       Type copy(rhs);    // Make a copy of the rhs.
       swap(copy);        // Swap the content of this and copy
       return *this;
  }

This can even be simplified. You can remove the explicit copy and use an implicit copy by taking the parameter by value:

  Type& operator=(Type copy)
  {
       swap(copy);        // Swap the content of this and copy
       return *this;
  }

Move by swap

The simplest way to implement a move is to use swap. You swap the content of the current object with the source object. The great thing about this is move semantics are supposed by exception safe as is swap. This also has the advantage of delayed destruction (the destruction of the object is delayed until the source object is destroyed; this gives an opportunity for the resources to be re-used).

So two things wrong with your move are that you forgot to mark them noexcept and, as mentioned above, you don't actually move because you use the named parameter.

Type(Type&& rhs) noexcept
   : member(default init)
   , ....
{
    swap(rhs);
}
Type& operator=(Type&& rhs) noexcept
{
    swap(ths);
    return *this;
}
void swap(Type& other) noexcept)
{
    using std::swap;
    swap(member,  other.member);
    ...
}
friend void swap(Type& lhs, Type& rhs) {
    lhs.swap(rhs);
}

Rule of Zero

If all the members already obey the rule of three/five and you don't have any special resource management to do in your class, then the rule of zero springs into effect.

The rule of zero means you don't need to define any constructors/assignment/destructors as the default compiler-generated versions will work perfectly for you.

Since your only data member is storage and this is a std::vector, then you are covered because std::vector obeys the rule of three and does all the correct resource management for you. So you don't need to specify any of the copy or move constructors as the compiler will correctly generate them for you.

Premature Optimization (is actually a pessimization)

This test

if (this != &rhs)
  storage = rhs.storage;

actually decreases performance in normal circumstances (I can't claim it always makes things worse but it usually does). You see, the problem is that self assignment is extremely rare (your code needs to work for it, but in reality it hardly happens).

This means you are adding a branch into your code (one of the worst things to do for optimizations) that is basically never taken. So you have to execute this test even though it is never taken. So you make the normal code path longer and it has a branch.

Now it does pay off if you do self assignment. But if you multiply the cost of checking the branch by the number of times per actual self assignment then subtract the cost of an extra copy then on average you are taking longer (thus a pessimization).

The normal way to implement assignment using the copy and swap always does a copy even though this is hideously more expensive on self assignment than it needs to be and it still pays off for normal usage of strings and vectors.

Pushing Efficiently

You sort of half do it.

void push(T item) noexcept {           // You make a copy of the parameter.
    storage.push_back(std::move(item));// Then you move the copy.
}

But what happens if I want to move an item into the stack? It could be more efficient than making a copy of it first. Also if you are going to copy delay the copy until the end.

// A moving push
void push(T&& item) noexcept {
    storage.push_back(std::move(item));
}
// A copy push
void push(T const& item) noexcept {
    storage.push_back(item);
}

The third choice we have is to build an item in place. Some large types may be easier to create in-place on the stack rather than pushing or moving them (say the constructor takes a single integer but creates a huge object that is expensive to create or move).

template<typename... P>  // template var arg (can be many parameters.
void push(P&&... args) {
    storage.emplace_back(std::forward<P>(args)...);
}

Top efficiency

When you call top() you always make a copy of the top item. But what about when you want to mutate the top item. Do you have to call item = top();pop();/*modify item*/;push(item) to mutate the top item?

Also it means you have to construct an object externally (a empty object) before you can copy into it. Not all types can have an empty object. Not all types are cheap to construct. Additionally copying the item from the stack may be expensive. So this is not a desirable interface.

I would suggest you simply return a reference to the top item. That way it can be mutated in place or if the user wants they can make a copy by assigning to a local variable.

This does mean that your top() can not return true/false on successes but I would suggest that should be its own function to test for empty anyway.

bool empty() const   {return storage.empty();}
T&       top()       {return storage.back();}
T const& top() const {return storage.back();}

Notice I don't even check to see if there is an item. This is because the calling code can check with a call to empty(). The reason is I don't want to check if there is no need to check and if there is a need to check then the user can call empty(). If you need a checked version then you can add a specific checked version.

This is exactly why most C++ containers have checked and unchecked version of the interface. You should not need to force a check (as that will be inefficient).

while(!stack.empty())
{
    std::cout << stack.top() << ",";   // No need to copy if I am printing.
    stack.pop();  // Should not force a check in pop.
                  // We already know from context there is an item in
                  // the stack.
}

Your original code would have looked like this:

T  tmp;                        // You have to build a tmp object.
                               // Is this expensive?
while(stack.top(tmp))          // Check here.
{
    std::cout << tmp << ", ";
    stack.pop();               // Another check here. Even though
                               // we know the stack is not empty at
                               // this point.
}

Some minor notes:

Usually user types and namespace begin with an upper case letter, while objects and functions begin with a lower case letter.

This makes user types easier to spot.

 Stack::Stack<Int>  aStack;

Also the type modifiers * and & are part of the type. So in C++ (unlike C) they are usually grouped with the type not the variable.

Stack(const Stack<T> &other)
// More normally written like:
Stack(const Stack<T>& other)
// Or like this:
Stack(Stack<T> const& other)

The standard comment

Please avoid using namespace X;

using namespace stack;

This is really bad idea and will get you into a lot of trouble in larger code bases. So it is best not to get into the habit of this. Prefer to always prefix your types with the namespace.

stack::Stack<Int>   aStack;

If you have an exceedingly long (or nested namespace) you can use a namespace alias to simplify it (I usually use short three letter aliases).

namespace TSt = ThorsAnvil::Stack;

TSt::Stack<Int>     aStack;

See Why is “using namespace std” considered bad practice?.
Best Answer

Using Rule of zero to rewrite:

namespace stack {

// Exception safe moveable stack.
// No constructors or detructors are needed as the members
// do all the work correctly.
template <typename T>
class Stack {
private:
  std::vector<T> storage;

public:

  bool empty()   const noexcept {return storage.empty();}
  T const& top() const noexcept { // still no exception looking at object is safe
      return storage.back();      // will not cause an exception.
  }                               // It may be undefined if you top an empty stack.
  T&       top()       noexcept {
      return storage.back();
  }

  void push(T const& item) {     // Adding an item is not exception safe.
      storage.push_back(item);   // As adding an item to a vector may
  }                              // cause reallocation.
  void push(T&& item) {
      storage.push_back(std::move(item));
  }

  void pop() noexcept {
      storage.pop_back();
  }
};


Should I use and loop, or just a loop? I have this code:
if(listObj.Any(x => x.id < 0))
{
    foreach(ModelClass item in listObj)
    {
        if(item.id < 0)
        {
            // code to create a new Obj in the database
        }
    }
}

But, should I use like this?
foreach(ModelClass item in listObj)
{
    if(item.id < 0)
    {
        // code to create a new Obj in the database
    }
}

NOTE: It's unusual to exist id < 0 (I use this to create a temp id for manipulation in the page), but there is a possibility.

Extra info that I found:

Query transformations are syntactic
IMPROVE YOUR LINQ WITH .ANY()
MSDN Documentation - Enumerable.Any Method

Use .Where extension method to filter the records you need:

foreach(ModelClass item in listObj.Where(x => x.id < 0))
{
    // code to create a new Obj in the database
}


Angular dropdown menu I've made a dropdown menu using angularJS, it works fine but I feel that I'm not following the DRY principle. Is there a way to combine this switch statement into something more efficient with less code? Instead of having 3 "cases" can I just use an OR operator inside of a data-ng-switch-when or something like that? This code will get bigger because it's quite a big project so code efficiency is key.

{{item}}

{{subitem}}

{{subitem}}

{{subitem}}

Here's my controller containing the menu items:

onlinePlatform.controller('onlinePlatformCtrl', function ($scope) {

    $scope.mainMenu = ['Startsida', 'Nyheter', 'Meddelanden', 'Mina sidor', 'Forum', 'Kalender', 'Verktyg', 'Hjälp'],
    $scope.subMenu = {
        messages: ['Inkorg', 'Skickat', 'Borttaget'],
        myPages: ['Mina kurser', 'Mitt schema', 'Klasslista', 'Anteckningar'],
        tools: ['Ladda ner Dreamspark produkter']
    }

});

Make your menus more uniform, then you won't have to do the ng-switch-ing.  So something like

$scope.menus = [
  {
    title: 'Startsida'
  },
  {
    title: 'Messages',
    items: ['Inkorg', ...]
  },
  {
    title: 'My Pages',
    items: ['Mina kurser', ...]
  },
  ...
];

Then check whether you have the items field and if so, render the items:

{{menu.title}}

{{subitem}}

It might be worthwhile to split the URLs from the titles as well.

This approach will break down somewhat at a point when the nesting level of your menu gets too deep, because you have to add more nested lists, but until then this will keep working.


HackerRank Code "Determining DNA Health" I am working on this problem: https://www.hackerrank.com/challenges/determining-dna-health/problem passing some test cases and time-out on others (no wrong answers).

The code is here:

import java.io.*;
import java.math.*;
import java.security.*;
import java.text.*;
import java.util.*;
import java.util.concurrent.*;
import java.util.regex.*;

public class Solution {

    private static final Scanner scanner = new Scanner(System.in);

    public static void main(String[] args) {
        int n = scanner.nextInt();
        scanner.skip("(\r\n|[\n\r\u2028\u2029\u0085])?");

        String[] genesItems = scanner.nextLine().split(" ");
        scanner.skip("(\r\n|[\n\r\u2028\u2029\u0085])?");

        Map<Integer, String> genes = new HashMap<>();
        for (int i = 0; i < n; i++) {
            genes.put(i, genesItems[i]);
        }

        String[] healthItems = scanner.nextLine().split(" ");
        scanner.skip("(\r\n|[\n\r\u2028\u2029\u0085])?");

        Map<Integer, String> health = new HashMap<>();
        for (int i = 0; i < n; i++) {
            health.put(i, healthItems[i]);
        }

        int s = scanner.nextInt();
        scanner.skip("(\r\n|[\n\r\u2028\u2029\u0085])?");

        long min = Long.MAX_VALUE;
        long max = Long.MIN_VALUE;

        for (int sItr = 0; sItr < s; sItr++) {
            String[] firstLastd = scanner.nextLine().split(" ");

            int first = Integer.parseInt(firstLastd[0]);
            int last = Integer.parseInt(firstLastd[1]);
            String d = firstLastd[2];

            long sum = 0;
            for (int i = first; i <= last; i++) {
                for (int index = d.indexOf(genes.get(i)); index >= 0; index = d.indexOf(genes.get(i), index + 1)) {
                    sum += Integer.parseInt(health.get(i));
                }
            }
            if (sum < min) {
                min = sum;
            }
            if (sum > max) {
                max = sum;
            }
        }

        System.out.println(min + " " + max);
        scanner.close();
    }
}

The Algorithm
This has a bad worst-case complexity,
for (int i = first; i <= last; i++) {
    for (int index = d.indexOf(genes.get(i)); index >= 0; index = d.indexOf(genes.get(i), index + 1)) {
        sum += Integer.parseInt(health.get(i));
    }
}

Consider a strand "aaaaaaaaaaaaaaaaaaaaaaa" and genes {"aaaaa", "aaaaa" ...
Now for every gene, at almost every position of the strand, there will be a string comparison. The amount of character comparisons is approximately TotalSize(genes) * strand.length, so that can easily lead to a TLE if the data is a little annoying, which of course it will be.
Here are a couple of other techniques you could try,

Building a suffix array and LCP array for the strand, so for every gene the number of occurrences in the strand can be counted in O(geneLength + log strandLength) (so without even visiting the occurrences!) by binary-searching for the start and end of the contiguous range in the SA that contains all those occurrences. A suffix trie is good as well but more dangerous for the memory footprint. SA and suffix trie can both be constructed in linear time (in the length of the string), but it is non-trivial to implement. The LCP array can be given as a secondary result of SA construction (depending on the algorithm) or otherwise created in linear time as well by using the SA.
Building a trie with the healthy genes, then go over the strand while descending down the trie, backing up as needed. This has a bunch of edge-cases that need to be considered. There are slow cases, for example if a gene is very big (half the length of the strand) and matches nearly everywhere (for example if the strand and gene are both only the same character), which I think is still a quadratic case. Some additional trickery may be able to work around that.

The solutions that I could find quickly seemed to use the second approach, but I think the first one would be a "safer" choice in terms of avoiding a quadratic worst-case complexity, though the second one seems useful enough in practice (it has been successfully used, after all).
Unnecessary Map
When the keys of a map are a dense and non-changing range of integers like this,
Map<Integer, String> genes = new HashMap<>();
for (int i = 0; i < n; i++) {
    genes.put(i, genesItems[i]);
}

Using an ArrayList or plain array is a little more efficient and maybe more convenient (the array genesItems is already available anyway). Though of course this does not significantly contribute to TLEs in competitive programming.
The skeleton code under the problem uses arrays.
Avoid re-parsing
The inner loop contains Integer.parseInt(health.get(i)), but that should usually be done up front when reading in the problem data. The skeleton code under the problem parses up front.


Optimizing a dynamic programming solution for "Oil Well" I'm trying to solve the Oil Well problem on Hackerrank using dynamic programming and it works. However, it times out for some of the test cases. I wanted to know how this program can be improved so that it runs faster.

The challenge is, given a matrix \$M\$ of size \$r \times c\$ (at most \$50 \times 50\$), where each \$M_{x,y}\$ is either 0 or 1, find the minimum cost path \$E\$ for a spanning tree that connects all the entries where \$M_{x,y} = 1\$.  The cost metric is based on ACME distance: \$\max(\left|x - x_1\right|, \left|y - y_1\right|)\$ where \$x, y\$ is the position of the existing wells, and \$x_1, y_1\$ is the position of the new well.

My basic idea is:

$$f(\mathrm{vertices}) = \min( \max(v_i \to \{\mathrm{vertices} - v_i\}) + f(\mathrm{vertices} - v_i))$$

for all vertices \$v_i\$.

The Hackerrank editorial shows a different logic but I'm trying to optimize my solution.

#include <cmath>
#include <cstdio>
#include <vector>
#include <iostream>
#include <algorithm>
#include <queue>
#include <set>
#include <climits>
#include <map>
#include <sstream>

using namespace std;
map<string,int> cache;
map<pair<int,int>,int> pd;
string has(vector<int>& vertices)
{
    ostringstream s;
    for(int i = 0; i < vertices.size(); i++)
    {
        s << vertices[i] << "|";
    }
    return s.str();
}
int dist(vector<int>& vertices)
{
    if(vertices.size() <= 1)
        return 0;
    string h = has(vertices);
    if(cache.count(h) != 0) {
        return cache[h];
    }
    int mind = INT_MAX;
    for(int i = 0; i < vertices.size(); i++) {
        int iv = vertices[i];
        int x = 0;
        for(int j = 0; j < vertices.size(); j++) {
            int ov = vertices[j];
            x = max(x,pd[make_pair(min(iv,ov),max(iv,ov))]);
        }
        vertices.erase(vertices.begin()+i);
        mind = min(x + dist(vertices), mind);
        vertices.insert(vertices.begin()+i,iv);
    }
    cache[h]=mind;
    return mind;
}
int main() {
    int r, c;
    cin >> r >> c;
    vector<pair<int,int>> vertices;
    int a;
    for(int i = 0; i < r; i++) {
        for(int j = 0; j < c; j++) {
            cin >> a;
            if(a) {
                vertices.push_back(make_pair(i,j));
            }
        }
    }
    vector<int> vv;
    for(int i = 0; i < vertices.size(); i++) {
        vv.push_back(i);
        for(int j = i+1; j < vertices.size();j++) {
            pair<int,int> ip = vertices[i];
            pair<int,int> op = vertices[j];
            int d = max(abs(ip.first - op.first),abs(ip.second - op.second));
            pd[make_pair(i,j)] = d;
        }
    }
    cout << dist(vv) << endl;
    return 0;
}

It is not possible to make this solution fast enough. The number of states in your dynamic programming is O(2^numberOfVertices)(the number of subsets of a set with numberOfVertices elements), which is 2^2500 in the worst case under given constraints. So a more efficient algorithm is required.


Script to redo a rails migration easily I need to redo a migration occasionally. The workflow is usually something like this:

find or ls | grep the migration using some keyword
copy the timestamp
paste the timestamp in bundle exec rake db:migrate:redo VERSION=123

As I hate repetitive work, I wrote a small bash script that allows running a migration using only a partial name. If there are many matches, it lists them all.

#!/usr/bin/env bash

err() {
  echo "redo: error: $1"
  echo "redo: usage: redo <migration_name>"
  exit 1
}

[[ -d ./db/migrate ]] || err "dir db/migrate not found"

migrations=$(find './db/migrate' -name '*.rb' | grep "$1" | sed 's#^\./db/migrate/##')
declare -A times
for m in $migrations; do
  time=$(echo "$m" | sed 's#\([0-9]*\)_.*.rb#\1#')
  name=$(echo "$m" | sed 's#[0-9]*_\(.*\)\.rb#\1#')
  times["$name"]="$time"
done

if [[ ${#times[@]} == 1 ]]; then
  name="${!times[@]}"
  ver="${times[$name]}"
  echo "redo: executing \`bundle exec rake db:migrate:redo VERSION=$ver\`"
  bundle exec rake db:migrate:redo VERSION="$ver"
  exit 0
fi

echo -e "redo: be more specific, candidates are:\n"
for k in "${!times[@]}"; do
  echo "$k : ${times[$k]}"
done
exit 1

I'd love to get feedback.

Error and initial check

I recommend to substitute $1 with $@, so that if you ever forget to quote it will still print the complete message.

err ()
{
  echo "redo: error: $@"
  echo "redo: usage: redo <migration_name>"
  exit 1
}

I am not sure why you put the usage in that function, since if there is no database, this does not help anyone.

[[ -d ./db/migrate ]] || err "dir db/migrate not found"

You also may want to check if you have too few/many arguments.

[[ -z $1 ]] && err No argument!
[[ $# -gt 1 ]] && err Two many arguments!

Finding the migrations

I recommend not using sed to truncate your pathnames. I also think it is completely superfluous since your trapping a few things later on and can simply ignore the prefix. You also do not need to use grep, find should be able to do that on his own. Hence instead of

migrations=$(find './db/migrate' -name '*.rb' | grep "$1" | sed 's#^\./db/migrate/##')

simply use

migrations=$(find './db/migrate' -name "*$1*.rb")

At this point I would check if it actually found anything, too.

[[ -z $migrations ]] && err I don't know what you are talking about.

Note that I omitted the quotes to make the point raised above.

Creating the array

I recommend not using times as a variable name, because there is a built-in bash command of the same name.
You can use avoid that echo |sed construct by using the build-in regular expressions comparison operator =~.

Since I don't know the exact form how the migrations are written, I can only assume that a migration comes in the form ./db/migrate/YYYYMMDDHHmmSS_name.rb. Where the timestamp is obviously just numbers, but name can be anything.

declare -A migration_times
for select_migration in $migrations; do
    pattern="([0-9]+)_([^\.]+)\.rb"
    if  [[ $select_migration =~ $pattern ]]; then
        migration_times["${BASH_REMATCH[2]}"]="${BASH_REMATCH[1]}"
    fi
done

That shortens your code quite considerably and should make it more effective since you are not calling another program.

Executing command or listing migrations

If the array does only have one element then you don't need to read the whole pair, you can simply use the value the complete array evaluates to, saving you the lines with reassigning the names. You can also skip the exit 0 statement because that would overwrite the exit status of your external call. You can tie the listing with an else statement to the conditional.
I also used printf to give it a nicer look if the length of the names vary %-10s has to be adjusted to your needs.

if [[ ${#migration_times[@]} == 1 ]]; then
    echo "redo: executing 'bundle exec rake db:migrate:redo VERSION=${migration_times[*]}'"
    # bundle exec rake db:migrate:redo VERSION=${migration_times[*]}
else
    echo "redo: be more specific, candidates are:"
    for element in "${!migration_times[@]}"; do
        printf "%-10s : %s\n" $element ${migration_times[$element]}
    done
    exit 1
fi

I think you can also easily migrate the changes from your answer to this review.


Most accurate isWindow function While I'm aware of various different means of checking if a variable is a Window object in JavaScript, I'm interested in writing a function that is more accurate and resilient.

For example, jQuery uses:

isWindow: function( obj ) {
    return obj != null && obj == obj.window;
}

which will return an incorrect result for:

var o = {};
o.window = o;
$.isWindow(o); //returns `true`, even though `o` is not a window object

For purposes of this question, assume that the browser's native functions and objects have not been modified at the time of initialization.

Currently my code is:

(function (w) {
    "use strict";
    var wStr;
    wStr = Object.prototype.toString.call(w);
    if (!w.isWindow) {
        w.isWindow = function (arg) {
            var e,
                str,
                self,
                hasSelf;

            //Safari returns `DOMWindow`
            //Chrome returns `global`
            //Firefox, Opera & IE9 return `Window`
            str = Object.prototype.toString.call(arg);
            switch (wStr) {
            case '[object DOMWindow]':
            case '[object Window]':
            case '[object global]':
                return str === wStr;
            }
            if ('self' in arg) {
                //`'self' in arg` is true if `'self'` is in `arg` or a prototype of `arg`
                hasSelf = arg.hasOwnProperty('self');
                //`hasSelf` is true only if `'self'` is in `arg`
                try {
                    if (hasSelf) {
                        self = arg.self;
                    }
                    delete arg.self;
                    if (hasSelf) {
                        arg.self = self;
                    }
                } catch (e) {
                    //IE 7&8 throw an error when `window.self` is deleted
                    return true;
                }
            }
            return false;
        };
    }
}(window));

A few assertions:

console.assert(isWindow(window), '"window" is a window');

var o = {};
o.self = o;
o.window = o;
console.assert(!isWindow(o), '"o" is not a window');

var w = window.open('about:blank');
console.assert(isWindow(w), '"w" is a window');

var x = window.open('http://www.example.com');
console.assert(isWindow(x), '"x" is a window');

What about taking advantage of the (stupid) fact that the context parameter inside a function invoked without call or apply will be the window?

mYApp.isWindow = function(obj) {
  if(!obj)
    return false
  return obj === (function() { return this})() //is this the window
     || obj.eval && obj.eval.call && obj.eval("this === (function(){ return this })()") //is that another window
     || false; //make undefined false
};

Seems to even work when wrapped in a with. That being said, you might have legitimate reasons to want to test for this but I can't think of any.

Edit: Added check for 'other' window. You could probably add some finagling to check that eval isn't a stub that always returns true. So if it returns true for example you could generate two random numbers, add them together, and then pass them into the eval string and have it add those numbers and confirm the result is indeed accurate. Let's be honest though, that's stupid, if someone wants to trick you that bad, they'll find a way.


Converting Markdown to HTML using Go I recently ported a blog of mine from Python to Go (to improve speed and performance) and while all is great so far, I'd like some help optimising the Markdown function to improve the general performance, maintenance and readability of the function.

I have this function because I write my blog articles in Markdown (.md) and then use Python Go to convert the raw Markdown to HTML for output as this saves me from having to write ridiculous amounts of HTML. (which can be tedious to say the least)

The Markdown function takes one argument (raw) which is a string that contains the raw Markdown (obtained using ioutil.ReadFile).

It then splits the Markdown by \n (removing the empty lines) and converts:

Bold and italic text (***,**,*)
Strikethrough text (~~blah blah blah~~)
Underscored text (__blah blah blah__)
Links ([https://example.com](Example Link))
Blockquotes (> sample quote by an important person)
Inline code (`abcccc`)
Headings (h1-h6)

While some of the supported features aren't exactly standard, this function works and outputs the expected result without any errors but being a new Go programmer and this being my first "real" Go project I'd like to know whether or not my code could be optimised for better performance, maintainability and readability.

Here a few questions I have regarding optimisation:

Would it make a difference to performance if I reduced the amount of imports?
Would it improve readability if I put the regexp.MustCompile functions into variables above the Markdown function?
Would it improve performance if I used individual regexes to convert Markdown headings instead of using for i := 6; i >= 1; i-- {...}?
If not, is there a way to convert i (an integer) to a string without using strconv.Itoa(i) (to help reduce the amount of imports)?

Here is my code:

package parse

import (
  "regexp"
  "strings"
  "strconv"
)

func Markdown(raw string) string {
  // ignore empty lines with "string.Split(...)"
  lines := strings.FieldsFunc(raw, func(c rune) bool {
    return c == '\n'
  })
  for i, line := range lines {
    // wrap bold and italic text in "<b>" and "<i>" elements
    line = regexp.MustCompile(`\*\*\*(.*?)\*\*\*`).ReplaceAllString(line, `<b><i>$1</i></b>`)
    line = regexp.MustCompile(`\*\*(.*?)\*\*`).ReplaceAllString(line, `<b>$1</b>`)
    line = regexp.MustCompile(`\*(.*?)\*`).ReplaceAllString(line, `<i>$1</i>`)
    // wrap strikethrough text in "<s>" tags
    line = regexp.MustCompile(`\~\~(.*?)\~\~`).ReplaceAllString(line, `<s>$1</s>`)
    // wrap underscored text in "<u>" tags
    line = regexp.MustCompile(`__(.*?)__`).ReplaceAllString(line, `<u>$1</u>`)
    // convert links to anchor tags
    line = regexp.MustCompile(`\[(.*?)\]\((.*?)\)[^\)]`).ReplaceAllString(line, `<a href="$2">$1</a>`)
    // escape and wrap blockquotes in "<blockquote>" tags
    line = regexp.MustCompile(`^\>(\s|)`).ReplaceAllString(line, `&gt;`)
    line = regexp.MustCompile(`\&gt\;(.*?)$`).ReplaceAllString(line, `<blockquote>$1</blockquote>`)
    // wrap the content of backticks inside of "<code>" tags
    line = regexp.MustCompile("`(.*?)`").ReplaceAllString(line, `<code>$1</code>`)
    // convert headings
    for i := 6; i >= 1; i-- {
      size, md_header := strconv.Itoa(i), strings.Repeat("#", i)
      line = regexp.MustCompile(`^` + md_header + `(\s|)(.*?)$`).ReplaceAllString(line, `<h` + size + `>$2</h` + size + `>`)
    }
    // update the line
    lines[i] = line
  }
  // return the joined lines
  return strings.Join(lines, "\n")
}

Performance

Regex

regex.MustCompile() is very expensive! Do not use this method inside a loop !

instead, define your regex as global variables only once:

var (
    boldItalicReg = regexp.MustCompile(`\*\*\*(.*?)\*\*\*`)
    boldReg       = regexp.MustCompile(`\*\*(.*?)\*\*`)
    ...
)

Headers

If a line is a header, it will start by a #. We can check for this before calling ReplaceAllString() 6 times ! All we need
 to do is to trim the line, and then check if it starts with #:

line = strings.TrimSpace(line)
if strings.HasPrefix(line, "#") {
    // convert headings
    ...
}

We could go further and unrolling the loop to avoid unecessary allocations:

count := strings.Count(line, "#")
switch count {
case 1:
    line = h1Reg.ReplaceAllString(line, `<h1>$2</h1>`)
case 2:
    ...
}

Use a scanner

The idiomatic way to read a file line by line in go is to use a scanner. It takes an io.Reader as parameters, so you can directly pass
your mardown file instead of converting it into a string first:

func NewMarkdown(input io.Reader) string {

    scanner := bufio.NewScanner(input)
    for scanner.Scan() {

        line := scanner.Text()
        ...
    }
}

Use []byte instead of string

In go, a string is a read-only slice of bytes. Working with strings is usually more expensive than working with slice of bytes,
so use []byte instead of strings when you can:

line := scanner.Bytes()
line = boldItalicReg.ReplaceAll(line, []byte(`<b><i>$1</i></b>`))

Write result to a bytes.Buffer

Instead of string.Join(), we can use a buffer to write each line in order to further reduce the number of allocations:

buf := bytes.NewBuffer(nil)
scanner := bufio.NewScanner(input)
for scanner.Scan() {

    line := scanner.Bytes()
    ...
    buf.Write(line)
    buf.WriteByte('\n')
}

return buf.String()

final code:

package parse

import (
    "bufio"
    "bytes"
    "io"
    "regexp"
)

var (
    boldItalicReg = regexp.MustCompile(`\*\*\*(.*?)\*\*\*`)
    boldReg       = regexp.MustCompile(`\*\*(.*?)\*\*`)
    italicReg     = regexp.MustCompile(`\*(.*?)\*`)
    strikeReg     = regexp.MustCompile(`\~\~(.*?)\~\~`)
    underscoreReg = regexp.MustCompile(`__(.*?)__`)
    anchorReg     = regexp.MustCompile(`\[(.*?)\]\((.*?)\)[^\)]`)
    escapeReg     = regexp.MustCompile(`^\>(\s|)`)
    blockquoteReg = regexp.MustCompile(`\&gt\;(.*?)$`)
    backtipReg    = regexp.MustCompile("`(.*?)`")

    h1Reg = regexp.MustCompile(`^#(\s|)(.*?)$`)
    h2Reg = regexp.MustCompile(`^##(\s|)(.*?)$`)
    h3Reg = regexp.MustCompile(`^###(\s|)(.*?)$`)
    h4Reg = regexp.MustCompile(`^####(\s|)(.*?)$`)
    h5Reg = regexp.MustCompile(`^#####(\s|)(.*?)$`)
    h6Reg = regexp.MustCompile(`^######(\s|)(.*?)$`)
)

func NewMarkdown(input io.Reader) string {

    buf := bytes.NewBuffer(nil)

    scanner := bufio.NewScanner(input)
    for scanner.Scan() {

        line := bytes.TrimSpace(scanner.Bytes())
        if len(line) == 0 {
            buf.WriteByte('\n')
            continue
        }

        // wrap bold and italic text in "<b>" and "<i>" elements
        line = boldItalicReg.ReplaceAll(line, []byte(`<b><i>$1</i></b>`))
        line = boldReg.ReplaceAll(line, []byte(`<b>$1</b>`))
        line = italicReg.ReplaceAll(line, []byte(`<i>$1</i>`))
        // wrap strikethrough text in "<s>" tags
        line = strikeReg.ReplaceAll(line, []byte(`<s>$1</s>`))
        // wrap underscored text in "<u>" tags
        line = underscoreReg.ReplaceAll(line, []byte(`<u>$1</u>`))
        // convert links to anchor tags
        line = anchorReg.ReplaceAll(line, []byte(`<a href="$2">$1</a>`))
        // escape and wrap blockquotes in "<blockquote>" tags
        line = escapeReg.ReplaceAll(line, []byte(`&gt;`))
        line = blockquoteReg.ReplaceAll(line, []byte(`<blockquote>$1</blockquote>`))
        // wrap the content of backticks inside of "<code>" tags
        line = backtipReg.ReplaceAll(line, []byte(`<code>$1</code>`))
        // convert headings
        if line[0] == '#' {

            count := bytes.Count(line, []byte(`#`))
            switch count {
            case 1:
                line = h1Reg.ReplaceAll(line, []byte(`<h1>$2</h1>`))
            case 2:
                line = h2Reg.ReplaceAll(line, []byte(`<h2>$2</h2>`))
            case 3:
                line = h3Reg.ReplaceAll(line, []byte(`<h3>$2</h3>`))
            case 4:
                line = h4Reg.ReplaceAll(line, []byte(`<h4>$2</h4>`))
            case 5:
                line = h5Reg.ReplaceAll(line, []byte(`<h5>$2</h5>`))
            case 6:
                line = h6Reg.ReplaceAll(line, []byte(`<h6>$2</h6>`))
            }
        }
        buf.Write(line)
        buf.WriteByte('\n')
    }
    return buf.String()
}

Benchmarks

I used the folowing code for benchmarks, on a 20kB md file:

func BenchmarkMarkdown(b *testing.B) {

    md, err := ioutil.ReadFile("README.md")
    if err != nil {
        b.Fail()
    }
    raw := string(md)
    b.ResetTimer()

    for n := 0; n < b.N; n++ {
        _ = Markdown(raw)
    }
}

func BenchmarkMarkdownNew(b *testing.B) {

    for n := 0; n < b.N; n++ {
        file, err := os.Open("README.md")
        if err != nil {
            b.Fail()
        }
        _ = NewMarkdown(file)
        file.Close()
    }
}

Results:

> go test -bench=. -benchmem

goos: linux
goarch: amd64
BenchmarkMarkdown-4               10     104990431 ns/op    364617427 B/op    493813 allocs/op
BenchmarkMarkdownNew-4          1000       1464745 ns/op      379376 B/op      11085 allocs/op

benchstat diff:

name        old time/op    new time/op    delta
Markdown-4     105ms ± 0%       1ms ± 0%   ~     (p=1.000 n=1+1)

name        old alloc/op   new alloc/op   delta
Markdown-4     365MB ± 0%       0MB ± 0%   ~     (p=1.000 n=1+1)

name        old allocs/op  new allocs/op  delta
Markdown-4      494k ± 0%       11k ± 0%   ~     (p=1.000 n=1+1)


Print "n" number of "hello world" I had come up with the following code:

hello_worlds 0 = putStr ""
hello_worlds x = do putStrLn "Hello World"
                    hello_worlds (x-1)
main = do
   n <- readLn :: IO Int
   hello_worlds n

I am aware that it is making one extra call to putStr.

Is there any better way of doing this? If so, please give some advice regarding why I shouldn't go this way.

Instead of using explicit recursion, you can build your solution out of standard Prelude functions. This frequently ends up being more readable when the functions you're writing are more complex as common patterns are expressed by familiar names instead of properties of your recursive step.
hello_worlds n = putStrLn $ unlines (replicate n "Hello World")

In this case we have n repetitions of the string "Hello World", joined by newlines, and printed to stdout.
All top-level function definitions should include type annotations as well. It's a good source of documentation, it serves as a good check that you and the type checker agree on what your function is supposed to be doing, and it often gives the type checker enough information that you don't need to use type annotations inside the body of a function. In this case, by giving a type to hello_worlds you don't need to say what the return value of readLn is, because it can be seen that the n in main must be an Int since it's the argument to hello_worlds.
hello_worlds :: Int -> IO ()
hello_worlds n = putStrLn $ unlines (replicate n "Hello World")

main :: IO ()
main = do
    n <- readLn
    hello_worlds n

The last small change I'd make is to rename hello_worlds to helloWorlds. Haskell style prefers CamelCase.


Combining multiple masses to obtain a desired total mass Here is my code. Let me describe what I do here. I have 7kg soaps, and 2kg soaps. And I have total kg that mean how many kg soaps I need. I should use 7kg soaps first, if there is no need I shouldn't use 2 kg soaps. If I don't use any 2kg soaps, it must return -1, if I use 2 kg soaps it must return the amount of how many of them I use. I have prepareCargoPacket(int 7kg soap, int 2 kg soap, int totalKg).
For example, prepareCargoPacket(2,2,16); return 1s since I use 2kg soaps one time. prepareCargoPacket(1,3,10); returns -1 because I can't get 10kg from these values, and I can't use 2kg soaps
package com.bit32.cargocalculation;

public class CargoCalculation {

/**
 * Returns how many 2Kgs soap we need to use
 *
 * @param count7Kg -> count of how many 7Kgs soaps we have
 * @param count2Kg -> count of how many 2Kgs soaps we have
 * @param totalKg  -> the needed Kg I have to get
 * @return
 */

public int prepareCargoPacket(int count7Kg, int count2Kg, int totalKg) {

int solution = -1; // if there is no answer, or no need for 2Kgs soaps, the answer is -1

// if we have 7Kg ones many more than totalKg, we should decrease it to get from 2Kgs.
if (7*count7Kg > totalKg) {
    return prepareCargoPacket(count7Kg-1, count2Kg, totalKg);

// if we have enough 7Kgs that are equal to totalKg, we will not use 2Kgs.
} else if (7*count7Kg == totalKg) {
    return solution; // solution = -1

// if we have 7Kgs less than totalKg
} else {

// currentTotalKg is equal to the amount of Kg that we can use for 2Kgs
    int currentTotalKg;
    for (int i = count7Kg; i>=0; i--) {

        currentTotalKg = totalKg - 7*i;

        if (currentTotalKg % 2 == 0 && count2Kg*2 >= currentTotalKg) {
            solution = currentTotalKg/2;
            break;

        }
        return solution;
    }

}

return solution;

}

First of all, your code doesn't actually work for some cases. The return statement in the for loop causes the loop to only ever run once. This means your function doesn't find a solution for prepareCargoPacket(2, 10, 12) (it tries with 1 7kg soap, fails the if check since 5 isn't a multiple of 2, then returns -1 without ever trying with no 7kg soaps). Removing that return fixes it.
Another thing I want to point out is that your way to cap the number of 7kg soaps is really not very efficient. If you need to make a total weight of 6kg when you have 200000 7kg soaps available, your function will call itself 200000 times. This is not ideal, for multiple reasons. Not only is it a lot of steps (each of which takes some amount of time), but the program will also internally keep track of each function call in the call stack, and that thing can only get so big before your program crashes with a java.lang.StackOverflowError.
A way around that is to notice that if you have more than totalKg / 7 7kg soaps, the total weight is already too high. So if you have too many 7kg soaps at the start, you don't need to keep trying with 1 less each time, because totalKg / 7 just gives you a nice upper bound.
To finish off with some nitpicking, I'm not a huge fan of repeating the weights of the soaps all over the place like that. If the person who wants this software comes up to you and says "Hi, sorry, we switched from 7kg soaps to 5kg soaps, can you update the program for us?" it would be annoying to replace the weight everywhere, and there'd be a risk of missing a place. A nicer approach could be replacing the numbers with constant variables, maybe something like final int HEAVY_WEIGHT = 7; and final int LIGHT_WEIGHT = 2;? That way, if the weights need changing, it only happens in one place. At that point I might also look into renaming the input parameters to something like heavyAvailable and lightAvailable to match those while I'm at it.
A few more thoughts
I looked some more, and came up with a few more comments.
Apart from the issue with there possibly being a lot of function calls, having the function call itself at all feels like it makes the logic a bit harder to follow. I'd like to restructure it so that that isn't necessary, with that initial check being used not to re-enter the function, but to determine where the loop starts.
Another, fairly minor issue is that this function keeps going for longer that it might have to even though it should know it won't find a solution. For example, if called as prepareCargoPacket(5, 0, 36) we get to the loop, notice we don't have enough 2kg soaps to reach the weight of 1kg that's missing. But then we continue to check if we can make 8kg instead. Which we can't since we have too few soaps. So we go on to check 15kg instead. But we can't make that either, so we check if we can make 22kg... Now imagine if instead of starting at 5 and 36 we'd started at something like 142,857,142 and 1,000,000,000.
Well, you don't have to imagine. I tested it, and it was still really fast. Never underestimate a computer's ability to do math quickly. Still, it can matter if the loop contains anything more complicated than a few basic math operators and a few ifs, so I'm bringing it up for future reference anyway. I also feel like switching the loop condition to take that into account also makes the code a bit cleaner, but it also arguably becomes a bit less obvious why it's written the way it is, so it might not actually be an improvement.
The solution variable doesn't seem entirely necessary to be honest. Each time the function returns, it either returns -1, or it changes solution for the first time and then immediately returns. At that point, I might get rid of that variable to save the reader from having to keep track of whether it's been updated. That's a matter of taste, though, there are definitely people who'd disagree with me on that.
Finally, since I've already done some nitpicking about variable names, I might as well complain about the last few. totalKg doesn't feel as descriptive as it could be. It and currentTotalKg are also a bit confusable, because "the total" and "the current total" don't really feel all that distinct to me - while that is sometimes the best way to describe two values, we might want to look for something that makes their purposes even more obvious. Going by what they're doing, totalKg might be more accurately described as the goalWeight or targetWeight, and since currentTotalKg is the amount we're missing to be able to reach that we might call that missingWeight or remainingWeight perhaps?
Taking all that into account, I might write something a bit like
package com.bit32.cargocalculation;

public class CargoCalculation {

    public static final int HEAVY_WEIGHT = 7;
    public static final int LIGHT_WEIGHT = 2;

    /**
    * Calculates how many lightweight soaps we need to use to reach exactly the target weight, if possible
    * If no lightweight soaps are required, or if the target weight can't be reached, returns -1
    *
    * @param heavyAvailable The number of heavy soaps that can be used
    * @param lightAvailable The number of light soaps that can be used
    * @param targetWeight The desired sum of the weights of all the soaps
    * @return
    */
    public int prepareCargoPacket(int heavyAvailable, int lightAvailable, int targetWeight) {
        int maxHeavyUsed;
        if (HEAVY_WEIGHT * heavyAvailable > targetWeight) {
            maxHeavyUsed = targetWeight / HEAVY_WEIGHT;
        } else {
            maxHeavyUsed = heavyAvailable;
        }

        // Loop until we find a solution, or until we know we can't
        for (int remainingWeight = targetWeight - HEAVY_WEIGHT * maxHeavyUsed;
             remainingWeight <= LIGHT_WEIGHT * lightAvailable;
             remainingWeight += HEAVY_WEIGHT) {
            if (remainingWeight == 0) {
                // Special case: We have to return -1 if we don't need light soaps
                return -1;
            } else if (remainingWeight % LIGHT_WEIGHT == 0 &&
                       lightAvailable * LIGHT_WEIGHT >= remainingWeight) {
                return remainingWeight / LIGHT_WEIGHT;
            }
        }

        // No solution
        return -1;
    }
}

Though as I said if you think it's clearer to write the loop as for (int i = maxHeavyUsed; i >= 0; i--) (and then define remainingWeight inside the loop), you can do that instead.