reusing Body in http.Requests (goproxy) Goproxy is a Go package implementing an HTTP proxy.
I am using it to store requests from a client (eg: a web browser) for further processing.

In an http.Request struct, the Body is an io.ReadCloser, it can only be read once.

The problem I faced is that I need to read the Body to store it, but still make it accessible to goproxy so that it can be sent to the server.

I solved it by reading the body with ioutil.ReadAll, and then creating a   ReadCloser with the byte slice I got. I then assign this ReadCloser to the request Body.

package main

import (
    "bytes"
    "flag"
    "fmt"
    "io/ioutil"
    "log"
    "net/http"
    "net/url"
    "regexp"
    "sync"

    "github.com/elazarl/goproxy"
)

var requests map[string][]Request
var mutex sync.Mutex

type Request struct {
    Method        string
    URL           url.URL
    Proto         string // "HTTP/1.0"
    ProtoMajor    int    // 1
    ProtoMinor    int    // 0
    Header        http.Header
    Body          []byte
    ContentLength int64
    Host          string
    PostForm      url.Values
    RequestURI    string
}

type RequestBody struct {
    *bytes.Reader
}

func (r RequestBody) Close() error {
    return nil
}

func storeRequest(r *http.Request, ctx *goproxy.ProxyCtx) (*http.Request, *http.Response) {
    body, err := ioutil.ReadAll(r.Body)
    if err != nil {
        // only print error for now
        log.Println(err)
        return r, nil
    }
    r.Body.Close()
    r.Body = RequestBody{bytes.NewReader(body)}
    fmt.Println(string(body))
    mutex.Lock()
    requests[r.URL.Host] = append(requests[r.URL.Host], Request{
        Method:        r.Method,
        URL:           *r.URL,
        Proto:         r.Proto,
        ProtoMajor:    r.ProtoMajor,
        ProtoMinor:    r.ProtoMinor,
        Header:        r.Header,
        Body:          body,
        ContentLength: r.ContentLength,
        Host:          r.Host,
        PostForm:      r.PostForm,
        RequestURI:    r.RequestURI,
    })
    mutex.Unlock()
    return r, nil
}

func main() {
    proxy := goproxy.NewProxyHttpServer()
    proxy.OnRequest(goproxy.ReqHostMatches(regexp.MustCompile("^.*$"))).
        HandleConnect(goproxy.AlwaysMitm)
    proxy.OnRequest().DoFunc(storeRequest)

    requests = make(map[string][]Request)

    addr := flag.String("addr", ":8080", "proxy listen address")
    flag.Parse()
    log.Fatal(http.ListenAndServe(*addr, proxy))
}

I would like to know if this is a correct way of solving this problem.

It is fine to read the body and replace it, although see ioutil.NopCloser.

However,

it does not make sense to duplicate the http.Request type. Re use it instead. If you need finer control, embed it.
take care to make true copy when passing around slices, maps and pointers.
the code is lacking structured factorization, make use of type definition

This is a revised version i suggest,

package main

import (
    "bytes"
    "flag"
    "io"
    "io/ioutil"
    "log"
    "net/http"
    "net/url"
    "regexp"
    "sync"

    "github.com/elazarl/goproxy"
)

type store struct {
    requests map[string][]http.Request
    sync.Mutex
}

func (s *store) Add(r *http.Request) {
    var c http.Request
    c.Method = r.Method
    c.Proto = r.Proto
    c.ProtoMajor = r.ProtoMajor
    c.ProtoMinor = r.ProtoMinor
    c.Host = r.Host
    c.RequestURI = r.RequestURI
    c.ContentLength = r.ContentLength
    c.URL = r.URL

    c.Header = http.Header{}
    for k, vv := range r.Header {
        for _, v := range vv {
            c.Header.Set(k, v)
        }
    }

    c.PostForm = url.Values{}
    for k, vv := range r.PostForm {
        for _, v := range vv {
            c.PostForm.Set(k, v)
        }
    }

    c.Form = url.Values{}
    for k, vv := range r.Form {
        for _, v := range vv {
            c.Form.Set(k, v)
        }
    }

    var b bytes.Buffer
    io.Copy(&b, r.Body)
    r.Body.Close()
    buf := b.Bytes()

    var b1 bytes.Buffer
    b1.Write(buf)
    r.Body = ioutil.NopCloser(&b1)

    c.Body = ioutil.NopCloser(bytes.NewBuffer(buf))

    key := r.URL.Host
    s.Lock()
    s.requests[key] = append(s.requests[key], c)
    s.Unlock()
}

type storeHandler struct {
    requests *store
}

func (s storeHandler) Handle(r *http.Request, ctx *goproxy.ProxyCtx) (*http.Request, *http.Response) {
    s.requests.Add(r)
    return r, nil
}

func main() {
    h := storeHandler{requests: &store{requests: map[string][]http.Request{}}}

    proxy := goproxy.NewProxyHttpServer()
    proxy.OnRequest(goproxy.ReqHostMatches(regexp.MustCompile("^.*$"))).
        HandleConnect(goproxy.AlwaysMitm)
    proxy.OnRequest().DoFunc(h.Handle)

    addr := flag.String("addr", ":8080", "proxy listen address")
    flag.Parse()
    log.Fatal(http.ListenAndServe(*addr, proxy))
}


Git tag release build files and reset to commit without This is part of a library that wants to release builded files (eg dist/*) when published to npm and bower. However, I don't want to commit those builded files for obvious reasons. Bower uses tags, and so I need tag to hold a version that contains builded files.

I didn't think twice, and that's what I came up with:

#!/bin/bash
[[ '' == $1 ]] && echo "Please provide patch, minor, major argument" && exit 1

# build dist files
gulp
# use npm to get the next semver (ie npm version patch increments patch number)
# --no-git-tag-version is to avoid any git interactions
newver=$(npm --no-git-tag-version version $1)

# dist files are ignored, adding them with the updated package.json
git add -f dist package.json

# commit and tag the new version with dist files
git commit -m $newver
git tag $newver

# publish on npm
npm publish

# reset the latest commit
git reset --hard HEAD~1

# issue the npm command again so that package.json gets updated
newver=$(npm --no-git-tag-version version $1)

# only add the package.json, dist files are gone
git add package.json

# commit and push tags
git commit -m $newver
git push --tags
git push

I'm wondering if this would be considered as a bad-practice, if there is something I could improve, or if someone has a better idea on how to do this. It's working great, but is having a tag that refers to an overridden commit a good idea?

It's a very nicely written script.

  It's working great, but is having a tag that refers to an overridden commit a good idea?

It doesn't sound great, but I don't have a better idea either.
In any case, I don't see any harm in doing this.
The content of these commits don't pollute the history because they are not really part of it.

If repository storage becomes a problem later,
you can always wipe out the obsolete tags.
(As you mentioned in a comment, this might not be such a good idea if package managers (like bower) rely on git tags for dependencies...)

Error handling

What if gulp fails? The script will happily go ahead and publish something broken or half-baked.
It would be good to add some error handling here, for example:

if ! gulp; then
    echo "Fatal: gulp failed, aborting..."
    exit 1
fi

Code duplication

This line appears twice:

newver=$(npm --no-git-tag-version version $1)

As it's a non-trivial command,
it would be good to put it in a function, for example:

set_newver() {
    newver=$(npm --no-git-tag-version version $1)
}

Naming

It's good to give command line parameters a proper name early on,
and refer to them by that name throughout the script,
instead of a meaningless $1. For example version.

This may be subjective, but I find newver a bit awkward,
and perhaps npm_version would be better. I leave that up to you.


Alien Name Generator (using blocks of consonants and vowels) This is my first script in Python, and I am looking for tips and constructive criticism about code style, whether there are more efficient ways of doing things, etc. (not as interested in tips about the method, more so the actual code).

Method:

Choose random no. blocks (between 2 & 6)
Decide whether to start vowel or consonant (random).
Generate all blocks of vowels (random vowels, vowel number between 1 & 3)
Generate all blocks of consonants (as above).
Merge together to create 'name'.

## Alien Name Generator 1.0
## 19/7/15
## Build names using alternating blocks of all consonants or vowels

import math
import random
random.seed(a=None, version=2)
print("Alien name generator:")

# Useful parameters:
NUM_NAMES = 100 # number of names to generate
mx_name_blocks = 5 # max number of blocks that can be used
vbl = 3 # max length of a single vowel block
cbl = 2 # max length of a single consonant block

filenametosave = "namelist5.txt"

## NOTE: Alphabet characters are 97 (a) - 122 (z)
# vowels are (96 +): 1,5,9,15,21
vowel_numbers = [97, 101, 105, 111, 117]
# consonant numbers:
consonant_list = list(range(97,123)) # start with full alphabet
for count2 in vowel_numbers: # remove vowels from alphabet
    consonant_list.remove(count2) # list of consonants

namelist = list() # store names from the main loop

### Main Loop (generates a single word each time):

for count in range (0,NUM_NAMES):

    # Random number of blocks to use:
    n_vcs = random.randint(2,mx_name_blocks)

    # Randomly start vowel or consonant:
    vc_start = int(round(random.random(),0)) # v = 0, c = 1

    # Work out number of vowel or consonant blocks:
    n_v = math.ceil(n_vcs/2) - vc_start*(n_vcs%2) # no. vowels
    n_c = n_vcs-n_v # no. consonants

    ### Generate vowel block list:

    vowel_block_list = list() # store all vowel blocks

    for count0 in range(1,1+n_v): # loop for each block

        vlength = random.randint(1,vbl) # random length of vowel block

        ### Generate single vowel blocks:

        v_block = list() # Store single vowel block

        for count1 in range(1,1+vlength): # loop for each individual vowel
            v1_val = chr(vowel_numbers[random.randint(0,4)]) # random vowel
            v_block.append(v1_val) # create list of vowels in single block

        string_block = ''.join(v_block) # join to make string

        vowel_block_list.append(string_block) # create list of all vowel blocks

    ### Generate consonant block list:

    const_block_list = list() # store all consonant blocks

    for count3 in range(1,1+n_c):
        clength = random.randint(1,cbl) # random length of block

        ### Generate single blocks:

        c_block = list() # store single block

        for count4 in range(1,1+clength):
            c1_val = chr(consonant_list[random.randint(0,20)]) # random character
            c_block.append(c1_val)

        cstring_block = ''.join(c_block)

        const_block_list.append(cstring_block) # list of consonant blocks

    ### Combine vowel and consonant blocks, alternating:

    if n_v > n_c:
        mixed = []
        for i in range(len(vowel_block_list)):
             mixed.append(vowel_block_list[i])
             if i < len(const_block_list):
                mixed.append(const_block_list[i])
    else:
        mixed = []
        for i in range(len(const_block_list)):
             mixed.append(const_block_list[i])
             if i < len(vowel_block_list):
                mixed.append(vowel_block_list[i])

    aName = ''.join(mixed).title()
    namelist.append(aName)

print("\n".join(namelist))

# WRITE TO FILE:
f = open(filenametosave,'w')
f.write("Alien name generator names: \n"+"\n".join(namelist))
f.close()

Example output:

Gque
Iiolyeeejuui
Hacbiieq
Bxau
Jyuu

All of your code is top-level, there are no functions and the only way to understand this programme is reading it all, also it is not tested so developing it was probably difficult.

Here is how I would go about it, with extensive comments.

"""
Block based alien name generator.
"""

This is a module level docstring, the first thing that gets displayed when the user types help(your_module) it usually describes concisely the purpose of the whole script.

import doctest
import itertools
import random
import string
import time

The Python Standard Library is extensive, many modules make devoloping programs easier.

MIN_BLOCK_LENGTH = 1
MAX_BLOCK_LENGTH = 2
MIN_BLOCKS_NUMBER = 2
MAX_BLOCKS_NUMBER = 3
VOWELS = "aeiou"
CONSONANTS = ''.join(i for i in string.ascii_lowercase if i not in  VOWELS)
TARGET_FILENAME = "nice_alien_names.txt"

These are module level constants, they either contain values that would be meaningless to change such as VOWELS or configuration options such as MAX_BLOCK_LENGTH that I modified as I like best short names. They are written SHOUTCASE and should not be changed (Python won't stop you from doing so though).

def random_string(length, pool):
    """
    >>> random.seed(0)
    >>> random_string(5, "abc")
    'bbabc'
    """
    return ''.join(random.choice(pool) for _ in range(length))

Now we get to the real meat. This is a function, an individual block of code with a name that performs one task and only that, return gives that value back into the programme. You may see the use of a generator expression, it can greatly shorten and simplify code. It is also tested, and the tests serve also as documentation.

def random_vowel_block():
    return random_string(random.randint(MIN_BLOCK_LENGTH, MAX_BLOCK_LENGTH), VOWELS)

def random_consonant_block():
    return random_string(random.randint(MIN_BLOCK_LENGTH, MAX_BLOCK_LENGTH), CONSONANTS)

Two small helper functions, to have function names near to the specification.

def alternate(func_1, func_2, times):
    """
    Returns a list containing `times` results
    of the two functions alternated.

    >>> list(alternate(lambda: 1, lambda: 2, 3))
    [1, 2, 1]
    """
    return list(itertools.chain(
        * zip( (func_1() for _ in range(times)),
               (func_2() for _ in range(times)))))[0:times]

This function is probably more complex than it should be ;P, anyhow docstring and test simplify understanding. As you can see I abstracted the concept of alternation into a higher order function (a function that takes functions as input) that I will use in the main alien_name function.

def alien_name():
    """
    Generates a random alien name by concatenating a random
    number (between MIN_BLOCKS_NUMBER and MAX_BLOCKS_NUMBER)
    of random all-vowel and all-consonant blocks, of which each one
    has a length between MIN_BLOCK_LENGTH and MAX_BLOCK_LENGTH.

    >>> random.seed(0)
    >>> alien_name()
    'aiqmiu'
    >>> alien_name()
    'wtaa'
    """
    name = ''.join(alternate(random_vowel_block, random_consonant_block,
                     random.randint(MIN_BLOCKS_NUMBER, MAX_BLOCKS_NUMBER)))
    return name if random.randint(0, 1) else ''.join(reversed(name))

The most important function, as you can see it is very small, as we have already defined all the building blocks needed. It reads almost as plain English. The last line implies that is a 50% chance of reversal to allow names starting with consonants.

def save_liked_ones():
    print("Enter nothing to ignore, `l` or `like` to save and `end` to terminate and save.\n")
    good_names = []
    while True:
        candidate_name = alien_name()
        choice = input("{}: ".format(candidate_name)).lower()
        if not choice:
            pass
        if choice.startswith('l'):
            good_names.append(candidate_name)
        if choice == 'end':
            with open(TARGET_FILENAME, 'a+') as f:
                f.write('\n'.join(good_names))
            return

I took the liberty of writing a user interface to avoid saving dull or uninteresting names, as you can see the logic is being performed all inside alien_name() this function only handles user interface.

if __name__ == "__main__":
    doctest.testmod()
    random.seed(time.time())
    save_liked_ones()

This runs the tests that you see in the docs and shows the user interface if this file is run as a script, importing it will only cause the functions to be defined. Please note that random.seed is needed as I seeded the Random Number Generator before for ease of testing.

And the full programme, in case you want to play and experiment with it:

"""
Block based alien name generator.
"""

import doctest
import itertools
import random
import string
import time

MIN_BLOCK_LENGTH = 1
MAX_BLOCK_LENGTH = 2
MIN_BLOCKS_NUMBER = 2
MAX_BLOCKS_NUMBER = 3
VOWELS = "aeiou"
CONSONANTS = ''.join(i for i in string.ascii_lowercase if i not in  VOWELS)
TARGET_FILENAME = "nice_alien_names.txt"

def random_string(length, pool):
    """
    >>> random.seed(0)
    >>> random_string(5, "abc")
    'bbabc'
    """
    return ''.join(random.choice(pool) for _ in range(length))

def random_vowel_block():
    return random_string(random.randint(MIN_BLOCK_LENGTH, MAX_BLOCK_LENGTH), VOWELS)

def random_consonant_block():
    return random_string(random.randint(MIN_BLOCK_LENGTH, MAX_BLOCK_LENGTH), CONSONANTS)

def alternate(func_1, func_2, times):
    """
    Returns a list containing `times` results
    of the two functions alternated.

    >>> list(alternate(lambda: 1, lambda: 2, 3))
    [1, 2, 1]
    """
    return list(itertools.chain(
        * zip( (func_1() for _ in range(times)),
               (func_2() for _ in range(times)))))[0:times]

def alien_name():
    """
    Generates a random alien name by concatenating a random
    number (between MIN_BLOCKS_NUMBER and MAX_BLOCKS_NUMBER)
    of random all-vowel and all-consonant blocks, of which each one
    has a length between MIN_BLOCK_LENGTH and MAX_BLOCK_LENGTH.

    >>> random.seed(0)
    >>> alien_name()
    'aiqmiu'
    >>> alien_name()
    'wtaa'
    """
    name = ''.join(alternate(random_vowel_block, random_consonant_block,
                     random.randint(MIN_BLOCKS_NUMBER, MAX_BLOCKS_NUMBER)))
    return name if random.randint(0, 1) else ''.join(reversed(name))

def save_liked_ones():
    print("Enter nothing to ignore, `l` or `like` to save and `end` to terminate and save.\n")
    good_names = []
    while True:
        candidate_name = alien_name()
        choice = input("{}: ".format(candidate_name)).lower()
        if not choice:
            pass
        if choice.startswith('l'):
            good_names.append(candidate_name)
        if choice == 'end':
            with open(TARGET_FILENAME, 'a+') as f:
                f.write('\n'.join(good_names))
            return

if __name__ == "__main__":
    doctest.testmod()
    random.seed(time.time())
    save_liked_ones()


Simplify the restructuring of json data I am trying to ad nested to some flat data, which is nested.

Basically this code works the following way:

"taglevel":1 tags should be key of the array
"taglevel":2 or higher tags should be nested within an array and not be duplicated in its' array
If no "taglevel":1 exists add, it to a generic "NoLevel_1" array

The code is still clunky and I feel there is a much cleaner way to achieve this.

import json
generic = []
result = []

for i in json_data:
    if any(d['taglevel'] == 1 for d in i['tag']):
        tag_data = {}
        tag_child = []
        for tag in i['tag']:
            if tag['taglevel'] == 1:
                tag_data['name'] = tag['name']
                tag_data['taglevel'] = 1
            else:
                tag_child.append(tag)
        filtered = {tuple((k, d[k]) for k in sorted(d) if k in ['name']): d for d in tag_child}
        tag_data['tag_child'] = list(filtered.values())
        if any(d['name'] == tag_data['name']  for d in result):
            for t in result:
                if t['name'] == tag_data['name']:
                    t['tag_child'] = t['tag_child'] + tag_child
                filtered = {tuple((k, d[k]) for k in sorted(d) if k in ['name']): d for d in t['tag_child']}
            t['tag_child'] = list(filtered.values())
        else:
            result.append(tag_data)
    else:
        for tag in i['tag']:
            generic.append(tag)
tag_data = {}
tag_data['name'] = 'NoLevel1'
tag_data['taglevel'] = 1
tag_data['tag_child'] = generic
result.append(tag_data)
print json.dumps(result, indent=4, sort_keys=True)

The data:

json_data = [{
        "title": "Random",
        "tag": [
            {
                "name": "Fruit",
                "taglevel": 1
            },
            {
                "name": "Apple",
                "taglevel": 2
            }
        ]
    },
    {
        "title": "Other",
        "tag": [
            {
                "name": "Fruit",
                "taglevel": 1
            },
            {
                "name": "Apple",
                "taglevel": 2
            }
        ]
    },
    {
        "title": "Words",
        "tag": [
            {
                "name": "Food",
                "taglevel": 2
            }
        ]
    },
        {
        "title": "That",
        "tag": [
            {
                "name": "Food",
                "taglevel": 2
            },
            {
                "name": "Apple",
                "taglevel": 2
            }
        ]
    }
]

Desired result

[
    {
        "name": "Fruit",
        "tag_child": [
            {
                "name": "Apple",
                "taglevel": 2
            }
        ],
        "taglevel": 1
    },
    {
        "name": "NoLevel_1",
        "tag_child": [
            {
                "name": "Food",
                "taglevel": 2
            },
            {
                "name": "Apple",
                "taglevel": 2
            }
        ],
        "taglevel": 1
    }
]

Well you may or may not considered this a simplification but this is how I would approach it.

You could use a set to handle the duplication part.

You can't store a dict in a set though so we need to create a tuple from the values. (It looks like you're doing something similar with filtered)

We then reformat result to get the desired final structure.

from collections import defaultdict

result = defaultdict(set)

for item in json_data:
    parent   = {'name': 'NoLevel_1'}
    children = []
    for tag in item['tag']:
        if tag['taglevel'] == 1:
            parent = tag
        else:
            children.append((tag['taglevel'], tag['name']))
    result[parent['name']].update(children)

result = [
    {
        'name': parent,
        'tag_child': [
            {'name': name, 'taglevel': taglevel} for taglevel, name in tags
        ],
        'taglevel': 1,
    } for parent, tags in result.items()
]

You could use next() and a list comprehension for the parent and child creation however it iterates the tags twice and may not be as "readable"

parent   = next((tag for tag in item['tag'] if tag['taglevel'] == 1), {'name': 'NoLevel_1'})
children = [(tag['taglevel'], tag['name']) for tag in item['tag'] if tag['taglevel'] != 1]


Converting a numerical string to the equivalent multipled by 100 Method:

Input: a string which represents an integer or a non-integer value.
Output: a string which represents the same value multiplied by 100.

Requirements:

No floating-point unless needed.
No leading 0s in the part left of the floating-point.
No trailing 0s in the part right of the floating-point.

I cannot use the Double.parseDouble method, because it sometimes yields an "ugly" result.

For example:

String.valueOf(Double.parseDouble("0.0071")*100)); // yields 0.7100000000000001
String.valueOf(Double.parseDouble("0.072" )*100)); // yields 7.199999999999999

Here is my code, and I would like to know if there are any cavities in it:

String MultiplyBy100(String number)
{
    StringBuilder sb = new StringBuilder();

    String[] parts = number.split("\\.");
    if (parts.length == 1)
        sb.append(parts[0]+"00");
    else if (parts[1].length() == 1)
        sb.append(parts[0]+parts[1]+"0");
    else if (parts[1].length() == 2)
        sb.append(parts[0]+parts[1]);
    else
        sb.append(parts[0]+parts[1].substring(0,2)+"."+parts[1].substring(2));

    if (sb.charAt(0) == '-')
    {
        while (sb.length() > 2 && sb.charAt(1) == '0' && sb.charAt(2) != '.')
            sb.deleteCharAt(1);
    }
    else
    {
        while (sb.length() > 1 && sb.charAt(0) == '0' && sb.charAt(1) != '.')
            sb.deleteCharAt(0);
    }

    return sb.toString();
}

It would be much easier with BigDecimals:

String input = "00007.880000";
BigDecimal hundred = BigDecimal.valueOf(100);
BigDecimal result = new BigDecimal(input).multiply(hundred).stripTrailingZeros();

System.out.println(result.toString()); // prints "788"

It works with big numbers well:

String input = "0000123456789123456789.123456789123456789123456789123456789880000";
BigDecimal hundred = BigDecimal.valueOf(100);
BigDecimal result = new BigDecimal(input).multiply(hundred).stripTrailingZeros();

// prints "12345678912345678912.345678912345678912345678912345678988"
System.out.println(result.toString());

Two readings:

Effective Java, 2nd Edition, Item 48: Avoid float and double if exact answers are required
Why not use Double or Float to represent currency?

The original code returns 788.0000 for 00007.880000 which I guess does not fulfill the specification.
Java developers usually start method names with lowercase letters.

  Methods should be verbs, in mixed case with the first letter lowercase, with the first letter of each internal word capitalized.

Source: Code Conventions for the Java TM Programming Language, 9 - Naming Conventions


Optimizing simple menu in console app I want to optimize code in my simple app that I'm writing to learn C#. I feel it's really badly optimized in terms of good coding practises. Could anyone take a look at it and give me a hint to make it look better?

public void mainMenu(BookList b)
        {
            bool repeat = false;
            Console.WriteLine("Welcome to Library.");
            Console.WriteLine("What do you want to do?");
            Console.WriteLine("1. Add a book.");
            Console.WriteLine("2. Delete book.");
            Console.WriteLine("3. Borrow a book.");
            Console.WriteLine("4. Return book.");
            Console.WriteLine("5. See list of available books.");
            Console.WriteLine("6. See list of all books.");
            Console.WriteLine("7. Exit");
            string userChoice = Console.ReadLine();
            switch  (userChoice)
            {
                case "1":
                    do
                    {
                        b.addBook();
                        Console.WriteLine("What do you want to do now?");
                        Console.WriteLine("Add another book? - Press 1.");
                        Console.WriteLine("Go back to main menu? - Press any other button.");
                        ConsoleKeyInfo keyPressed;
                        keyPressed = Console.ReadKey();
                        if (keyPressed.Key == ConsoleKey.D1 || keyPressed.Key == ConsoleKey.NumPad1)
                        {
                            repeat = true;
                            Console.Clear();
                        }
                        else
                        {
                            repeat = false;
                        }
                    }
                    while (repeat == true);
                    Console.Clear();
                    this.mainMenu(b);
                    break;
                case "2":
                    b.deleteBook();
                    break;
                case "3":
                    b.borrowBook();
                    break;
                case "4":
                    b.returnBook();
                    break;
                case "5":
                    b.displayBooks();
                    break;

            }
        }

I wanted to add repeating an action in addbook() function, but mainMenu is method of different class so I figured it out this way.

Your application is a nice candidate for the command pattern. With a few changes you can make it easily extendable and maintainable. Here's how it goes:

First you need an interface for such a command. Each command needs at least a descritpiton and it must be able to do something:

interface ICommand
{
    string Description { get; }
    void Execute(BookList books);
}

Next we can start implementing the commands:

class AddBookCommand : ICommand
{
    public string Description => "Add a book.";
    public void Execute(BookList books) { }
}

class DeleteBookCommand : ICommand
{
    public string Description => "Delete book.";
    public void Execute(BookList books) { }
}

class ExitCommand : ICommand
{
    public string Description => "Exit.";
    public void Execute(BookList books) { Environment.Exit(0); }
}

Then in your Main you could create all the commands:

var commands = new ICommand[]
{
    new AddBookCommand(),
    new DeleteBookCommand(),
    new ExitCommand()
};

a list of books:

var books = new BookList();

and the main loop:

while (true)
{
    Console.WriteLine("Welcome to Library.");
    Console.WriteLine("What do you want to do?");

    // This loop creates a list of commands:
    for (int i = 0; i < commands.Length; i++)
    {
        Console.WriteLine("{0}. {1}", i + 1, commands[i].Description);
    }

    // Read until the input is valid.
    var userChoice = string.Empty;
    var commandIndex = -1;
    do
    {
        userChoice = Console.ReadLine();
    }
    while (!int.TryParse(userChoice, out commandIndex) || commandIndex > commands.Length);

    // Execute the command.
    commands[commandIndex - 1].Execute(books);
}

This is just a start. You still need to add console maintenance like .Clear  some more error checking etc.


Concatenate variable number of strings I am learning C on my own. This is not an assignment. I am trying to implement a function that takes a variable numbers of strings and concatenate them in a new string. It works, but I feel it is not elegant and would like to improve it.

For instance, I don't like how I am using n nor having to decrement argc twice, and I don't like having to use both length and total_length. On a more general level, I would like to know if there is a way to parse the variable arguments only once, although it does not seem to be possible unless allocating a fixed number of character or calling malloc multiple times (which I want to avoid).

/*
 * concat_strings - concatenate a variable number of strings
 * usage:
 * const *message = concat_strings(4, "Error running", argv[0], " - ", "Action create: needs name and email");
 */
char *concat_strings(int argc, ...)
{
    va_list ap;

    /* calculate the total number of characters in all the strings */
    va_start(ap, argc);
    int n = argc;
    int l = 0;
    while(n--)
        l += strlen(va_arg(ap, char*));
    va_end(ap);

    /* allocate memory */
    const size_t sizeof_char = sizeof(char);
    char *joined = malloc(sizeof_char * (l + 1));

    /* copy strings to destination */
    va_start(ap, argc);
    size_t length = 0;
    size_t total_length = 0;
    char *s;
    while(argc--) {
        s = va_arg(ap, char*);
        memcpy(joined + total_length, s, sizeof_char * (length = strlen(s)));
        total_length += length;
    }
    *(joined + total_length) = '\0';
    va_end(ap);

    return joined;
}

Calling strlen() twice on each string is wasteful in time.

Current code make 3 trips down each string, once for length, once for copying, once to determine new length.

    l += strlen(va_arg(ap, char*));
    ...
    memcpy(joined + total_length, s, sizeof_char * (length = strlen(s)));
    total_length += length;

The following runs down each string twice, once for length determination and a second for copying.  It "parses" the variable arguments only once, but then does need to save those results somewhere.

char* concat_n(size_t n, ...) {
  va_list ap;
  size_t size = 1;

  // Allocate memory or for small `n`, use VLA or fixed array.
  struct {
    size_t len;
    const char *ptr;
  } str[n];

  va_start(ap, n);
  for (size_t i = 0; i < n; i++) {
    str[i].ptr = va_arg(ap, const char *);
    str[i].len = strlen(str[i].ptr);
    if (str[i].len > SIZE_MAX - size) {  // Detect overflow in length
      va_end(ap);
      return NULL;
    }
    size += str[i].len;
  }
  va_end(ap);

  char *dest = malloc(size);
  if (dest) {
    char *p = dest;
    for (size_t i = 0; i < n; i++) {
      memcpy(p, str[i].ptr, str[i].len);
      p += str[i].len;
    }
    *p = '\0';
  }
  return dest;
}

An alternative to saving the lengths and recomputing strlen() is to use some non-standard function like char *strpcpy() which copies and returns the pointer to the null character.  As if this is faster than other approaches, YMMV.  IMO, memcpy() is the best to speed code along and since the length was  compute prior, take advantage of that even if managing the saving of those prior results is a minor extra burden.

int is not the best type for array index nor size math.  size_t is the Goldilocks type, neither too wide, nor too narrow.

sizeof_char appears artificial.  Alternatives:

// char *joined = malloc(sizeof_char * (l + 1));
char *joined = malloc(l + 1);
// or
char *joined = malloc(sizeof *joined * (l + 1));

Defensive programming.  What happens if user passed a negative value for argc?   (Answer: a very long loop).

// Problem code when `argc < 0`
char *concat_strings(int argc, ...) {
  ...
  int n = argc;
  while(n--) {

Suggest either size_t or > 0

char *concat_strings(size_t argc, ...) {
  ...
  size_t n = argc;
  while(n--) {

or

char *concat_strings(int argc, ...) {
  ...
  int n = argc;
  while(n-- > 0) {

Check allocation

char *joined = malloc(l + 1);
if (joined == NULL) return NULL;

argc, though not a reserved name, has special meaning.  Suggest a different variable name like string_count


Is there a better way to retrieve variables from class method Currently my code works but I wanted to know if I'm going about this the right way. I want to be able to individually call variables from method 'data_by_name' in 'otherfile.py' . At the moment I import 'otherfile.py' and put the elements into a list within 'main.py' and use tuple unpacking to give each element a variable name.

Could I go about this a better way?

otherfile.py

import requests
import json

API_KEY = "XXX"
PROJECT_TOKEN = "XXX"
RUN_TOKEN = "XXX"

class Data:
    def __init__(self, api_key, project_token):
        self.api_key = api_key
        self.project_token = project_token
        self.params = {"api_key":api_key}
        self.get_data()

    def get_data(self):
        r = requests.get('https://www.parsehub.com/api/v2/projects/PROJECT_TOKEN/last_ready_run/data', params={"api_key": API_KEY})
        self.data = json.loads(r.text)

    def data_by_name(self,country):

        data = self.data['country']
        for content in data:
            if content['Cname'].lower() == country.lower():
                print(content)
                name = content['Cname']
                pop = content['population']
                popRank = content['popRank']
                growth = content['growthRate']
                per = content['worldPer']
                area = content['area']
                capital = content['capital']
                region = content['region']
                gpd = content['gpd']
        return(name,pop,popRank,growth,per,area,capital,region,gpd)

if __name__ == '__main__':
    data = Data(API_KEY,PROJECT_TOKEN)
    data.data_by_name('Russia')

main.py

from ParsehubCode import Data,API_KEY,PROJECT_TOKEN

country = Data(API_KEY,PROJECT_TOKEN)
test = country.data_by_name("china")
list1 = []
for element in test:
    list1.append(element)
for x in list1:
    A,B,C,D,E,F,G,H,I = list1
print(A,B,C)

country.json

{
 "country": [
  {
   "Cname": "China",
   "name": "China Population 2020 (Live)",
   "population": "1,438,719,354",
   "popRank": "1",
   "growthRate": "0.39%",
   "worldPer": "18.47%",
   "area": "9,706,961 km²",
   "capital": "Beijing",
   "region": "Asia",
   "gpd": "$10,746.78"
  }
{

Instead of

list1 = []
for element in test:
    list1.append(element)
for x in list1:
    A,B,C,D,E,F,G,H,I = list1

you could simply write

A,B,C,D,E,F,G,H,I = test

Regardless, here's a slightly different approach.

Assuming that the input json structure is { 'country': [{ }, ..., { }] },
I'd slightly modify self.data:

 self.data = {country['Cname'].lower(): country
              for country in json.loads(r.text)['country']}

then access this dictionary whenever required. Possibly by overwriting __getitem__:

def __getitem__(self, key):
    return self.data[key.lower()]

Then you can access the inner dictionary using [ ]:

GlobalDataLoader(API_KEY,PROJECT_TOKEN)['china']
GlobalDataLoader(API_KEY,PROJECT_TOKEN)['china']['population']

Here I've renamed the Data class: I don't expect the constructor of a class named Data to access the internet, and the data seems to be global.
Still, I'd remove the self.get_data() call from __init__.

On the other hand, if you are not planning to reuse the (API_KEY, PROJECT_TOKEN) for a specific GlobalDataLoader instance, then I wouldn't store these in the instances. Instead do something like

data = GlobalDataLoader(project_secrets)

where

class GlobalDataLoader:
    def __init__(self, project_secrets):
        self.load_data(project_secrets)

    def load_data(self, project_secrets):
        url = f"https://www.parsehub.com/api/v2/projects/{project_secrets['project_token']}/last_ready_run/data"
        r = requests.get(url, params={"api_key": project_secrets['api_key']})
        self.data = {country['Cname'].lower(): country
                     for country in json.loads(r.text)['country']}

    def __getitem__(self, key):
        return self.data[key.lower()]

That being said, don't hard-code secrets into the source code.

API_KEY = "XXX"
PROJECT_TOKEN = "XXX"
RUN_TOKEN = "XXX"

You don't want this as a habit when you start using version control systems. Store these in a separate file. If you have to store that in a VCS, then encrypt it (e.g. use gitcrypt).

To reuse the idea of a wrapped dictionary, you could write a

class ProjectSecretsLoader:
    def __init__(self, config_file_path):
        self.load_config(config_file_path)

    def load_config(self, config_file_path):
        self.config_data = dict() # TODO

    def __getitem__(self, key):
        return self.config_data[key]

then do

config_file_path = "..."
project_secrets = ProjectSecretsLoader(config_file_path)
data = GlobalDataLoader(project_secrets)

Last, but not least requests.get might fail, json.loads might fail. The result of json.loads might not have the structure you expect, or might be missing the country you will eventually try to look up, etc. In these cases specific exceptions will be raised, which ideally should be handled, otherwise the program will crash.


Setting a page number from the URL, with fallbacks I have the following code, but it somehow looks a bit redundant.
As I am new to ColdFusion, I am not sure how to optimize it. Can someone suggest an alternative?

<cfparam name="url.idInfopage" default="0">
<cfif  not isNumeric(url.idInfopage)><cfset url.idInfopage= 0></cfif>

<cfif isDefined("stElement.idInfopage") and isNumeric(stElement.idInfopage)>
    <cfset idInfopage= stElement.idInfopage>
    <cfif  idInfopage is ""><cfset idInfopage= 0></cfif>
<cfelse>
    <cfset idInfopage= url.idInfopage>
</cfif>

You could re-write it something like this:

<cfif StructKeyExists(stElement,'idInfoPage') AND isNumeric(stElement.idInfopage)>
    <cfset idInfopage = stElement.idInfopage />

<cfelseif StructKeyExists(Url,'idInfoPage') AND isNumeric(Url.idInfopage)>
    <cfset idInfopage = Url.idInfopage />

<cfelse>
    <cfset idInfopage = 0 />

</cfif>

Notes on the changes:

StructKeyExists is generally recommended over isDefined.
isNumeric wont return true for empty string.

If you have a lot of code like this, it might make sense to create a function, something like:

<cfset idInfopage = getNumberOrZero('idInfopage',stElement,Url) />

Then implemented as:

<cffunction name="getNumberOrZero" returntype="numeric" output=false
    hint="Loops through structs (args 2..n) looking for number; else return 0">
    <cfargument name="Key" type="String" required />
    <cfset var i = 0 />

    <cfloop index="i" from=2 to=#ArrayLen(Arguments)#>
        <cfif StructKeyExists(Arguments[i],Arguments.Key)
            AND isNumeric(Arguments[i][Arguments.Key])
            >
            <cfreturn Arguments[i][Arguments.Key] />
        </cfif>
    </cfloop>

    <cfreturn 0 />
</cffunction>


Efficiently determine every factor of a given natural number It's not hard to find all of the factors of a given natural number. Below is a pretty fast python function that can do just that:
(Modified version of Steinar Lima's function, https://stackoverflow.com/a/19578818/5946921)

import itertools as it

def factor(n):
    step = 2 if n % 2 else 1
    factors = ([i, n//i] for i in range(1, int(sqrt(n))+1, step) if not n % i)
    return sorted(set(it.chain.from_iterable(factors)))

I am looking to make a function that can do this more efficiently for very large numbers. I have come up with the following function that does just that:

import itertools as it

def large_factor(n):
    primes = []
    ranges = []
    for prime, factors in it.groupby(primeFactor(n), lambda x: x):
        primes.append(prime)
        ranges.append(range(len(tuple(factors)) + 1))
    primes = primes[::-1]
    ranges = ranges[::-1]
    z = it.product(*(x for x in ranges))
    return sorted(product(a ** b for a, b in zip(primes, q)) for q in z)

I am looking for ways to improve this function. I want it to return the factors in ascending order, without duplicates. Originally, I was hoping I'd be able to have the entire thing be a lazy function (no eager evaluation at all). Unfortunately, I haven't been able to figure out a good way to do this. My approach right now is to get the prime factors from n, and a list of ranges [0, #primes factors + 1). These lists are then reversed, as this makes sorting easier later. The list of ranges is used to generate tuples that contain the powers that are applied to the prime numbers in the last line.

The following two functions are used in the large_factor() function:

import math

def primeFactor(n):
    ''' Generates prime factors of n'''
    if n <= 1: return
    prime = next((x for x in range(2, math.ceil(math.sqrt(n))+1) if n%x == 0), n)
    yield prime
    yield from primeFactor(n//prime)

from functools import reduce
from operator import mul

def product(iterable):
    ''' The product of the items in n '''
    return reduce(mul, iterable)

I've tested these functions with timeit, and found that factor() was significantly faster on small values on n, but on large values of n, large_factor() was significantly faster. I'm looking for any kind of optimization that can make large_factor() faster on smaller, or (especially) larger numbers, as well as any other suggestions you might have.

def primeFactor(n):
    ''' Generates prime factors of n'''
    if n <= 1: return
    prime = next((x for x in range(2, math.ceil(math.sqrt(n))+1) if n%x == 0), n)
    yield prime
    yield from primeFactor(n//prime)

For a start, consider that after yielding prime this can never yield a number less than prime, but the recursive call still starts again from 2. So one optimisation would be

def primeFactor(n):
    ''' Generates prime factors of n'''
    return primeFactorInner(n, 2)

def primeFactorInner(n, min):
    if n < min: return
    prime = next((x for x in range(min, math.ceil(math.sqrt(n))+1) if n%x == 0), n)
    yield prime
    yield from primeFactorInner(n//prime, prime)

Next, that range with an implicit step of 1 is a bit overkill. Once you've finished returning 2s you could just step through odd numbers. At this point I'm tempted to say that it would be convenient to change the return value to already do the grouping, and have an auxiliary method to get the power of a prime in n.

def primePower(n, p): return 0 if n%p > 0 else 1 + primePower(n//p, p)

Then we can write

def primeFactorInner(n, min):
    if n < min: return
    if min == 2:
        a = primePower(n, 2)
        if a > 0:
            yield (2, a)
            yield from primeFactorInner(n // (2 ** a), 3)
            return
    prime = next((x for x in range(min, math.ceil(math.sqrt(n))+1, 2) if n%x == 0), n)
    a = primePower(n, prime)
    yield (prime, a)
    yield from primeFactorInner(n // (prime ** a), prime + 2)

Now, if we carry that argument to extremes we would only want to loop over primes. So we could split into two phases:

for p in enumeratePrimes(math.floor(sqrt(n)) + 1):
    a = primePower(n, p)
    if (a > 0):
        yield (p, a)
        n = n // (p ** a)
        if n == 1:
            # Early abort
            return
if n > 1:
    yield (n, 1)

enumeratePrimes would probably best be implemented with a simple sieve.

  I want it to return the factors in ascending order, without
  duplicates. Originally, I was hoping I'd be able to have the entire
  thing be a lazy function (no eager evaluation at all). Unfortunately,
  I haven't been able to figure out a good way to do this.

The short answer is a priority queue. Initially you insert 1. When you pop and yield a factor, you push certain multiples of the factor. The mildly interesting part is ensuring that you only push each number once. I've found the Java code where I did a variant on this, and ported it to Python:

import heapq

def divisors(n):
    primes = [(1, 1)] + list(primeFactors(n))
    q = [(1, 0, 1)]
    while len(q) > 0:
        # d is the divisor
        # i is the index of its largest "prime" in primes
        # a is the exponent of that "prime"
        (d, i, a) = heapq.heappop(q)
        yield d
        if a < primes[i][1]:
            heapq.heappush(q, (d * primes[i][0], i, a + 1))
        if i + 1 < len(primes):
            heapq.heappush(q, (d * primes[i + 1][0], i + 1, 1))
            # The condition i > 0 is to avoid duplicates arising because
            # d == d // primes[0][0]
            if i > 0 and a == 1:
                heapq.heappush(q, (d // primes[i][0] * primes[i + 1][0], i + 1, 1))

I'm not really a Pythonista, so although I suspect that you can make this even lazier by avoiding the list(), I'll leave that to someone who knows the language better than me.


Completely static builder implementation CustomClass.class

Description

The CustomClass is created with init(), which returns a CustomBuilder saving both as static fields.

public class CustomClass {

    private static CustomClass customClass;
    private static CustomBuilder customBuilder;

    public static CustomBuilder init(){
        customClass = new CustomClass();
        return customBuilder = new CustomBuilder();
    }

    public static CustomClass customClass() {
        return customClass;
    }

    public static void setMethodA() {
        customBuilder.methodA();
    }

    public static void setMethodB() {
        customBuilder.methodB();
    }

    public static class CustomBuilder {

        public CustomBuilder methodA() {
            return this;
        }

        public CustomBuilder methodB() {
            return this;
        }

        public CustomClass build() {
            return customClass();
        }
    }
}

UsageActivity.class

Description

The UsageActivity calls init(),methodA() then build() within its onCreate(savedInstanceState). When something() is called it runs CustomClass.setMethodB().

    public class UsageActivity extends AppCompatActivity {

    @Override
    protected void onCreate(@Nullable Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        CustomClass.init().methodA().build();
    }

    protected void something(){
        CustomClass.setMethodB();
    }
}

Reasoning

Once the custom class' initial method is called in the activities onCreate, all its methods are static and accessible to the activity and its fragments.

As long as I handle data saving and lifecycles, it makes the library more flexible and creates less local variable management.

I'm not sure if this is the best way to go about it however. It all works as far as I can tell.

Does anyone see any issues or glaring mistakes?

Thank you.

I'm afraid and sorry to say that your code does not make a lot of sense.

A Builder is supposed to build an object. As such it replaces the constructor. That builders allow for chained method calls using return this is some syntactic sugar that's frequently used for builders. However, return this doesn't turn an arbitrary class into a Builder.

A Builder is used when it adds value over a regular constructor call. There are various reasons why one would introduce a Builder, to name a few:

The constructor call would be too complex or confusing.
A set of similar objects shall be created which differ in only a few elements.
To allow multiple method calls until the object is created while still retaining immutability for the final object.

The general expectation is that neither Builders nor classes created by them are Singletons, and that Builders and the constructed objects are independent of each other, that is, that they do not have side effects on each other.
In other words, one can create two Builders, and those two Builders would have nothing in common but being Builders for the same class. And then call build() on each of them, and end up with two different objects of possibly the same class but otherwise unrelated with each other.

The source of this is the massive over-use of static on non-final fields. Using static for fields without making them final is almost always a mistake.

More confusing things are

static Setters
Setters which have side effects on something else, something seemingly unrelated

Also, have you ever thought of the testability of the code in question?


Real time graph simulation of dice tossing When tossing a dice many many time all numbers tend to appear the same number of times, but if the number of throws is small then some numbers may appear more or less often than others, even considerably if the sample size is small enough.

This program shows a updating in real time graph of SIDES bars (6 in default), each presenting how many times a number has appeared so far.

Please ignore that the numbers on the X axis are wrong, that is a minor bug-fix for later, all of the visualization is there.

My code feels a bit messy, please help in organization and simplification:

import random

import numpy as np
import matplotlib.pyplot as plt
import matplotlib.animation as animation

SIDES = 6
HEIGHT = 100

def update_results(results):
    die_toss = random.randint(0, SIDES - 1)
    return [x + (1 if i == die_toss else 0) for i, x in enumerate(results)]

def plot_points(ps):
    plt.scatter(*zip(*ps))

def animate(frameno):
    global x
    x = update_results(x)
    n, _ = np.histogram(x, SIDES)
    n = x
    for rect, h in zip(patches, n):
        rect.set_height(h)
    print(x, max(x) / float(min(x)))
    return patches

if __name__ == "__main__":
    x = [ 1 for _ in range(SIDES) ]

    fig, ax = plt.subplots()
    n, bins, patches = plt.hist(x, SIDES, normed=1, facecolor='green', alpha=0.75)

    frames = 500
    ani = animation.FuncAnimation(fig, animate)

    axes = plt.gca()
    axes.set_ylim([0, HEIGHT])

    plt.show()

    plt.show()

    for i in range(100):
        plt.bar(range(SIDES), x)
        x =update_results(x)
        print(x)
        plt.show()
        plt.clf()

You are doing a lot of unneeded stuff. I completely deleted the for loop in the main part as well as the second plt.show. I also removed the plot_points function. None of this changed the functionality of the program, as far as I can tell.

matplotlib.animation.FuncAnimation takes an optional fargs argument which contains arguments to be passed on to the animate function. So this gets rid of needing global variables.

I made update_results modify x. This makes the update_results function obsolete, because now we only need to increment the element at the index chosen by the dice, which we can inline in the animate function.

I made the starting array start with all zeros, which is more realistic. Because of this I had to include a min(x) or 1 in animate to ensure that if the minimum is 0, 1 is used. I used the fact that [0] * 3 == [0, 0, 0] to simplify the initialization.

I also made the y limit fix you suggested by updating it in the animate function and made it obvious that the first parameter of the animate function is unused by using _ for it.

import random

import numpy as np
import matplotlib.pyplot as plt
import matplotlib.animation as animation

SIDES = 6

def animate(_, x, patches):
    die_toss = random.randrange(SIDES)
    x[die_toss] += 1
    for rect, h in zip(patches, x):
        rect.set_height(h)
    print(x, max(x) / float(min(x) or 1))
    axes = plt.gca()
    axes.set_ylim([0, max(x) + 1])
    return patches

def main():
    x = [0] * SIDES

    fig = plt.figure()
    n, bins, patches = plt.hist(
        x, SIDES, facecolor='green', alpha=0.75)

    frames = 500
    ani = animation.FuncAnimation(fig, animate, fargs=(x, patches))
    plt.show()

if __name__ == "__main__":
    main()


array_mask, Seeking alternative code I would really love to hear your critiques on:

Code quality
Code clarity
More efficient alternative code

Code:

function array_mask(array $source, array $filter){
    foreach ($source as $key => $value){
        if (!isset($filter[$key])){
            if (array_search($key, $filter) !== false){
                continue;
            }
            unset($source[$key]);
        } else {
            if (is_array($source[$key])){
                $source[$key] = array_mask($source[$key], $filter[$key]);
            }
        }
    }
    return $source;
}

$source = array(
    'page' => array(
        'q' => 'alkflsdj',
        'x' => 1,
        9   => 'm'
    ),
    'news' => 7
);
$filter = array(
    'page' => array(
        9
    ),
    'news'
);

$result = array_mask($source, $filter);
var_dump($result);

Result:

array
  'page' =>
    array
      9 => string 'm' (length=1)
  'news' => int 7

You can use built-in functions instead. Like this for example:

function array_mask(array $source, array $filter)
{
    $source = array_intersect_key($source, $filter);

    foreach ($source as $key => $value)
        if (is_array($value))
            $source[$key] = array_mask($source[$key], $filter[$key]);

    return $source;
}

$source = array(
    'page' => array(
        'q' => 'alkflsdj',
        'x' => 1,
        9   => 'm'
    ),
    'news' => 7
);
$filter = array(
    'page' => array(
        9 => ''
    ),
    'news' => ''
);

var_dump (array_mask($source, $filter));


Find the longest unique string in the input list Given a list, find the longest non-repeating (in other words I mean unique) string in the input list. Looking for code review, pointers on best practices, optimizations etc.

public final class LongestUniqueWord {

    /*
     * do not initialize this class
     */
    private LongestUniqueWord( ) {}

    /**
     * Returns the longest non-repeating(unique) word in the list.
     * If each word repeats then return null.
     *
     *
     * @param words the list of words of a news paper.
     * @return      the longest unique word.
     * @throws  IllegalArgumentException  if input size is 0.
     * @throws  NPE                       if the input is null.
     */
    public static String longestUniqueWord (List<String> words) {
        if (words.size() == 0) throw new IllegalArgumentException("The array should not be empty.");

        final Map<String, Boolean> uniqueWords = new HashMap<String, Boolean>();

        for (String word : words) {
            if (uniqueWords.containsKey(word)) {
                uniqueWords.put(word, false);
            } else {
                uniqueWords.put(word, true);
            }
        }

        int maxLength = 0;
        String longestUniqueWord = null;
        for (Entry<String, Boolean> entry : uniqueWords.entrySet()) {
            if (entry.getValue() && entry.getKey().length() > maxLength) {
                longestUniqueWord = entry.getKey();
                maxLength = longestUniqueWord.length();
            }
        }

        return longestUniqueWord;
    }

    public static void main(String[] args) {
        List<String> newsPaper = new ArrayList<String>();
        newsPaper.add("Jack");
        newsPaper.add("In");
        newsPaper.add("In");
        newsPaper.add("The");
        newsPaper.add("The");
        newsPaper.add("Box");

        System.out.println("Expected: Jack, Actual: "  + longestUniqueWord(newsPaper));

        newsPaper.add("Jack");
        System.out.println("Expected: Box, Actual: " + longestUniqueWord(newsPaper));
    }

}

I'm on my phone so can only give pointers.

In the first loop you can use Map.get and switch on its value:

null: put True and add to wordsByLength
True: put False and remove from wordsByLength
False: do nothing

Map<Integer, Set<String>> wordsByLength tracks unique word sets, one per word length. It maps word length to a set of unique words. Whenever a new word appears in 1 above, add it to the set. When it is seen again for the first time in 2, remove it.

Next sort those entries by their keys (word length) in reverse and pull the first word from the first set.

Note that you may end up with ties of same-length words. The standard set will return an undefined "first" word. That may be okay.


Implementation of Prim's algorithm in C++ Could anyone comment on what could be done better and if I made any mistakes?

#include <iostream>
#include <vector>
#include <algorithm>

using namespace std;

typedef pair<pair<int, int>, int> Edge; //a, b, length
// A---------B
//   length

vector<Edge> prims_algo(vector<vector<int>> graph, int number_of_nodes){
    vector<int> unvisited;
    vector<int> visited;
    vector<Edge> result;

    //mark first as visited and mark the rest as unvisited
    for (int i = 1; i < number_of_nodes; i++)
        unvisited.push_back(i);
    visited.push_back(0);

    while (!unvisited.empty()) {

        vector<Edge> edges_with_lengths;
        //put all edges (with their lengths) from nodes that are in visited
        for(auto node : visited) {
            for (int sec_node = 0; sec_node < number_of_nodes; sec_node++) {
                if (graph[node][sec_node] > 0 && find(unvisited.begin(), unvisited.end(), sec_node) != unvisited.end()) {
                    //add if there is connection and second node is not visited yet
                    Edge e = make_pair(make_pair(node, sec_node), graph[node][sec_node]);
                    edges_with_lengths.push_back(e);
                }
            }
        }

        //find the shortest edge
        pair<pair<int, int>, int> the_shortest;
        the_shortest = edges_with_lengths.front();
        for(auto i: edges_with_lengths){
            if(the_shortest.second > i.second)
                the_shortest = i;
        }

        //add the shortest path to the result
        result.push_back(the_shortest);

        //remove a node that the shortest edge leads to
        unvisited.erase(remove(unvisited.begin(), unvisited.end(), the_shortest.first.second), unvisited.end());

        //mark this node as visited
        visited.push_back(the_shortest.first.second);
    };
    return result;
}

int main() {
    /*
       2    3
   (0)--(1)--(2)
    |   / \   |
   6| 8/   \5 |7
    | /     \ |
   (3)-------(4)
         9          */
    vector<vector<int>> graph =
                     {{{0, 2, 0, 6, 0},
                       {2, 0, 3, 8, 5},
                       {0, 3, 0, 0, 7},
                       {6, 8, 0, 0, 9},
                       {0, 5, 7, 9, 0},
    }};

    vector<Edge> result = prims_algo(graph, 5);
    for(auto i:result){
        cout<<"EDGE [ " <<i.first.first<<", "<<i.first.second<<"], length: "<<i.second<<endl;
    }

    return 0;
}

Code looks OK.

I would suggest you to do some class instead of Edge:

//typedef pair<pair<int, int>, int> Edge; //a, b, length
struct Edge{
   int a;
   int b;
   int length;
};

Also I suggest to have some predefined type for vector<Edge>

using EdgeContainer = vector<Edge>;

You have duplication near //find the shortest edge, you probably need Edge there

Here the_shortest is probably not initialized. Also you probably might use something from <algorithm> (but I will do it in same way as you).

    //find the shortest edge
    pair<pair<int, int>, int> the_shortest;
    the_shortest = edges_with_lengths.front();
    for(auto i: edges_with_lengths){
        if(the_shortest.second > i.second)
            the_shortest = i;
    }

Another probably micro optimization would be to write ++i in loops:

//for (int i = 1; i < number_of_nodes; i++)
for (int i = 1; i < number_of_nodes; ++i){
   // ...
}

Also in C++11 main() does not need to return 0;.


Renaming all files/folders in first children folders but not sub-children Let's say I have this folder tree:

├──FolderParent
  ├── FolderA
      ├── FolderAA_IMG             -- should be renamed
          ├── file_IMG_AA1.txt     -- should NOT be renamed
          ├── file_VID_AA1.txt     -- should NOT be renamed
      ├── file_IMG_A1.txt          -- should be renamed
      ├── file_PANO_A1.txt         -- should be renamed
  ├── FolderB
      ├── file_IMG_B1.txt          -- should be renamed
      ├── file_PANO_B1.txt         -- should be renamed

As you can see, only the files/folders in the first children folder should be renamed, not the other ones. I follow this base code and then I added a second loop above but I wonder if the double for loop is the right way to go.

import os

# custom var
path= r"C:\Users\user\FolderParent"

# other cvar
number_modified_files= 0

# get all path of subfolder
all_subfolders = [f.path for f in os.scandir(path) if f.is_dir() ]
print(all_subfolders)

for folder in all_subfolders:
    # set the path to the folder: otherwise the rename file won't work
    os.chdir(folder)
    #won't rename files in subfolder but will rename folder in the path
    for filename in os.listdir(folder):
        print("new file:", filename)
        if "IMG_" in filename:
            os.rename(filename, filename.replace('IMG_', ''))
            number_modified_files +=1
        elif "PANO_" in filename:
            os.rename(filename, filename.replace('PANO_', ''))
            number_modified_files +=1
        elif "VID_" in filename:
            os.rename(filename, filename.replace('VID_', ''))
            number_modified_files +=1

print(f"End : {number_modified_file} files renamed")

You should rethink your solution in terms of regexes:

#!/usr/bin/env python3

import re
from os import scandir, rename, path

def rename_children(parent):
    n_renamed = 0
    re_fname = re.compile('(IMG|PANO|VID)_')

    for child_dir in scandir(parent):
        if child_dir.is_dir():
            for child in scandir(child_dir):
                renamed = re_fname.sub('', child.name)
                if renamed != child.name:
                    new_path = path.join(child_dir.path, renamed)
                    print(f'Renaming {child.path} to {new_path}')
                    rename(child.path, new_path)
                    n_renamed += 1
    print(f'{n_renamed} files renamed')

Note the following changes:

Only one if to check whether the regex matches
Use scandir instead of listdir
Do not call chdir; there's no point
Don't call replace; the pattern check and the replacement operation can be combined by using sub
Don't store a list of all_subfolders; simply iterate over the results


Checking if the entries of a map exist in another map I need to check if a map is subset of another map. For this, I have written the following code:

#include <string>
#include <iostream>
#include <map>

using namespace std;

/**
 * Checks if rhs is a subset of lhs
 */
template<class Map>
bool map_compare(Map &lhs, Map &rhs)
{
    if(rhs.size() > lhs.size() || rhs.empty()) return false;
    typename Map::iterator litr = lhs.begin();
    for (typename Map::iterator ritr = rhs.begin(); ritr != rhs.end(); ritr++)
    {
        if(litr == lhs.end() && ritr != rhs.end()) return false;
        while (litr != lhs.end())
        {
            if (litr->first == ritr->first)
            {
                if (litr->second == ritr->second)
                {
                    litr++;
                    break;
                }
                return false;
            }
            else
            {
                litr++;
            }
        }
    }
    return true;
}

int main()
{
    map<int, string> a, b;

    a[0] = "0";
    a[1] = "1";

    b[0] = "0";

    cout << "b ⊆ a? " << map_compare(a, b) << " (should be 1)\n";

    b[1] = "1";
    cout << "b ⊆ a? " << map_compare(a, b) << " (should be 1)\n";

    b[2] = "2";
    cout << "b ⊆ a? " << map_compare(a, b) << " (should be 0)\n";
}

I am looking for another implementation of map_compare which is either shorter or more efficient.

I believe you will find the standard library has the appropriate functions:
Includes

Includes tests whether one sorted range includes another sorted range. That is, it returns true if and only if, for every element in [first2, last2), an equivalent element [1] is also present in [first1, last1) [2]. Both [first1, last1) and [first2, last2) must be sorted in ascending order.
The two versions of includes differ in how they define whether one element is less than another. The first version compares objects using operator<, and the second compares objects using the function object comp.

You can see all the algorithms here
Your code would look like this:
#include <algorithm>
#include <iostream>
#include <map>

int main()
{
    std::map<int, std::string> a;
    std::map<int, std::string> b;

    a[0] = "0";
    a[1] = "1";

    b[0] = "0";

    std::cout << "b ⊆ a? " << std::includes(a.begin(), a.end(), b.begin(), b.end()) << " (should be 1)\n";

    b[1] = "1";
    std::cout << "b ⊆ a? " << std::includes(a.begin(), a.end(), b.begin(), b.end()) << " (should be 1)\n";

    b[2] = "2";
    std::cout << "b ⊆ a? " << std::includes(a.begin(), a.end(), b.begin(), b.end()) << " (should be 0)\n";
}


Thread-safe Iterator wrapper Aside: This code assumes a class Option that can be Some or None to represent the presence (Some) or absence (None) of the value it contains.  Haskell calls it Maybe.

Here is my first pass at wrapping Iterator in a thread-safe way:

public synchronized Option<T> nextBlocking() {
    return iter.hasNext() ? Option.of(iter.next())
                          : Option.none();
}

I don't like that it's synchronized - it requires locking in order to work across multiple threads.  On the other hand, if we remove the word synchronized, one thread could call iter.hasNext() which returns true (e.g. for the last item in the Iterator), then a second thread calls iter.next() before the first thread does, using up that last item and causing the first thread to get a NoSuchElementException when it finally calls iter.next().

The following isn't going to win any object-oriented style points, but I think it mostly works around this issue in a non-blocking way:

public Option<T> nextNonBlocking() {
    try {
         return Option.of(iter.next());
    } catch (NoSuchElementException nsee) {
        if(iter.hasNext()) {
            // Oops, exception was thrown for reason other than end-of-iterator.
            // Rethrow that exception.
            throw nsee;
        }
    }
    return Option.none();
}

That could still fail if a nsee is thrown for some reason before end-of-iterator, then another thread reaches the true end-of-iterator before the first thread calls iter.hasNext().  While possible, this situation probably represents a coding failure in the underlying Iterable.  It's very tempting to trade this tiny weakness for a non-blocking iterator wrapper.

Looking at your first code example, it's somewhat sensible. The synchronized method creates a "happens before" memory barrier, and as a result, all threads see the same version of the data at that point in the code.

There is a vulnerability in that code, though. The synchronized method means that your lock is publicly accessible. Anyone can synchronize on your wrapper, and create a conflicting lock, perhaps creating a "denial of service" situation, where you can never get in to your method because some other thread has your instance locked. This is rare, but it does happen in some programs.

About your second block of code.... this is just plain broken from a multi-threaded perspective:

public Option<T> nextNonBlocking() {
    try {
         return Option.of(iter.next());
    } catch (NoSuchElementException nsee) {
        if(iter.hasNext()) {
            // Oops, exception was thrown for reason other than end-of-iterator.
            // Rethrow that exception.
            throw nsee;
        }
    }
    return Option.none();
}

That code will seldom produce what you expect in an actual multi-threaded situation. For a start, it reverses the logic of the hasNext() and next() calls. You should call hasNext first. Secondly, in an actual multi-core, especially in a multi-memory-controller computer (multi-socket), you will run in to stale memory accesses. It is possible for all threads to iterate through all members of the iterator, as they will all be seeing different "caches" of the iterator.

The second block is just "plain wrong".

As an aside, Java 8 now has an Optional class. You should use it.


Design a main function which determines which secondary function to be used I am writing an R package. The package consists of a main function and lots of auxiliary functions that perform different calculations.

The main function

The idea behind the package is to make the user use only the main function and through it select which auxiliary function to use. This would imply that only the main function is exported while the auxiliary functions are not.
This is the way I am currently implementing the main function:

#' Main function
#' @export
#'
#' Current solution. Note that the parameter fun is a character
main_function <- function(fun,a,b){
    if (fun == "square"){square(a,b)}
    else if (fun == "cube") {cube(a,b)}
    else {stop("Function not found")}
}

Auxiliary functions

Auxiliary functions take all the same two parameters.

## Auxiliary functions (not to be exported, to be used only by the main function: the user should not access these functions)
square <- function(a,b){a^2+b}
cube <- function(a,b){a^3+b}

This is how I am currently solving this problem, it works fine, however I have the feeling there must be a better way or some sort of "best practice" for such cases. Is there not? Perhaps using substitute of lazyeval. Sadly I couldn't use either. I would like to avoid using a character as an argument in the main function and perhaps use the full name of the function such as main_function(f1,2,3). Could you please hint me in other possible directions? I would like for this code to be as close as possible to the best practices involved in creating an R package.

You could use match.arg and/or match.fun. They are both pretty standard approaches.

match.arg matches an input string to a pre-defined list. It allows for partial matches and handles the error in case it can't find a match.
match.fun is like a smart get for doing "the right thing" depending on the input.

Using match.arg:

square <- function(a, b) sprintf("square(%s, %s)", a, b)
cube   <- function(a, b) sprintf("cube(%s, %s)", a, b)
other  <- function(a, b) sprintf("other(%s, %s)", a, b)

main <- function(a, b, fun = c("square", "cube"), ...) {
   fun <- match.arg(fun)

   # Then, you could do
   # switch(method, square = square(a, b, ...),
   #                cube   = cube(a, b, ...))
   # or use match.fun:

   fun <- match.fun(fun)
   fun(a = a, b = b, ...)
}

main(a = 1, b = 2, fun = "square")
# [1] "square(1, 2)"
main(a = 1, b = 2, fun = square)
# Error in match.arg(fun) : 'arg' must be NULL or a character vector
main(a = 1, b = 2, fun = "cube")
# [1] "cube(1, 2)"
main(a = 1, b = 2, fun = "sq")  # partial matching
# [1] "square(1, 2)"
main(a = 1, b = 2)
# [1] "square(1, 2)"
main(a = 1, b = 2, fun = "other")
# Error in match.arg(fun) : 'arg' should be one of “square”, “cube”

Using match.fun:

main <- function(a, b, fun, ...) {
   print(fun)
   fun <- match.fun(fun)
   fun(a = a, b = b, ...)
}

main(a = 1, b = 2, fun = "square")
# [1] "square"
# [1] "square(1, 2)"
main(a = 1, b = 2, fun = `square`)
function(a, b) sprintf("square(%s, %s)", a, b)
# [1] "square(1, 2)"
main(a = 1, b = 2, fun = cube)
function(a, b) sprintf("cube(%s, %s)", a, b)
# [1] "cube(1, 2)"
main(a = 1, b = 2, fun = cub)   # no partial matching...
# Error in print(fun) : object 'cub' not found
main(a = 1, b = 2, fun = other)
# function(a, b) sprintf("other(%s, %s)", a, b)
# [1] "other(1, 2)"

As you can see, the two approaches behave a little differently as they may or may not:

allow functions as inputs (e.g. square instead of "square")
allow partial matching of function names
easily restrict the functions that can be used

So it will be up to you to decide based on your requirements.

Note: I have added ... to all your functions in case, in the future, your functions allow differing arguments. As you mentioned, your functions currently all only require a and b so you can get rid of , ... everywhere if you wish.


Multiple inheritance pattern for vehicle information I wanted an easy way to augment objects, by adding functionality from any other object(s). More importantly, I needed a way to augment the object from multiple sources in a clean one-line solution.

The inheritance is done with this:

function extend(proto, args){
    this[proto.id] = Object.create(proto);
    proto.constructor.call(this[proto.id], args);
}

extend.call(this, vehicle, args); // one-liner

I can invoke as many objects as needed; with this pattern it's easy to swap and change the prototype chain, so I could just put the above extend code in the vehicle object, if that's where I want augmentation.

It's now easy to create a car by pulling in whatever you need:

extend.call(this, vehicle, args);
extend.call(this, sunroof, args);
extend.call(this, tyres, args);
extend.call(this, wings, args);
extend.call(this, rocket, args);
etc...

Questions:

Plausibility: is it flawed in some way?
Optimisation: can I enhance the pattern?
clunky: I had to create an id property for each object, so the extend function knows how to create a property on the executing context. It seems like a hack. Is there a way to get a prototype name from a named constructor?

var manufacturer = {
    id:'manufacturer',
    constructor : function (args) {
        this.boss = args.boss || 'Bernie Ecclestone';
      this.country = args.country || 'UK';
        return this;
    }
};

var vehicle = {
    id:'vehicle',
    constructor : function (args) {
        this.colour = args.colour || 'blue';
      this.wheels = args.wheels || 2;
        extend.call(this, manufacturer, args);
        return this;
    }
};

var driver = {
    id:'driver',
    constructor : function (args) {
        this.name = args.name || 'John';
        return this;
    },
    info : function () { console.log(this.name); }
};

var engine = {
    id:'engine',
    constructor : function (args) {
        this.type = args.type || 'V6';
        this.fuel = args.fuel || 'petrol';
        return this;
    },
    tune : function () {
        this.type = 'super-charged';
        this.fuel = 'ethanol';
        console.log('Car now ' + this.type + ' with ' + this.fuel);
    }
};

var car = {
    id:'car',
    constructor : function (args) {
        extend.call(this, vehicle, args);
      extend.call(this, driver, args);
        extend.call(this, engine, args);
        return this;
    },
    info : function () {
        console.log('boss: ' + this.vehicle.manufacturer.boss);
        console.log('country: ' + this.vehicle.manufacturer.country);
        console.log('driver: ' + this.driver.name);
        console.log('colour: ' + this.vehicle.colour);
      console.log('wheels: ' + this.vehicle.wheels);
      console.log('type: '   + this.engine.type);
      console.log('fuel: '   + this.engine.fuel);
        console.log('\n');
  }
};

function extend(proto, args){
  this[proto.id] = Object.create(proto);
  proto.constructor.call(this[proto.id], args);
}

var ferrari = Object.create(car).constructor({
    boss: 'Maurizio Arrivabene',
    country:'Italy',
    name: 'Steve',
    colour: 'red',
    wheels: 4,
    type:'100cc',
    fuel:'diesel'
});

var lotus = Object.create(car).constructor({
    name: 'Jenson Button'
});

var mclaren = Object.create(car).constructor({
  type:'hybrid',
    fuel:'battery/petrol'
});

ferrari.engine.tune();
ferrari.info();
/*
Car now super-charged with ethanol
boss: Maurizio Arrivabene
country: Italy
driver: Steve
colour: red
wheels: 4
type: super-charged
fuel: ethanol
*/

lotus.info();
/*
boss: Bernie Ecclestone
country: UK
driver: Jenson Button
colour: blue
wheels: 2
type: V6
fuel: petrol
*/

mclaren.info();
/*
boss: Bernie Ecclestone
country: UK
driver: John
colour: blue
wheels: 2
type: hybrid
fuel: battery/petrol
*/

Very interesting concept, I could see this turning into a "Pimp my Ride" game. A few things before this is sent off to gamers:
OOP
It seems like you've kind of misunderstood what OOP is about. As it is, you have a lot of variables (not objects) which then are smushed together into a final concept (the car).
If you want this to be more OOP oriented, here's some things I suggest:
Get rid of all the variables, except car. Keep this, as it will represent your "object". Instead of the manufacturer having his own variable (who knows, maybe some car will be custom built or have various manufacturers), use getters and setters to define a property of car. Picture it this way:
Car object {
    Initialize the object here

    get manufacturer() {

    }
    set manufacturer(string or array or object) {

    }
}

Now your code might get a big here if you implement each detail with a getter and setter. Sometimes you can manage this, other times not, which then you might break things apart: have the manufacturer get/set in an engine object, and the car will only have the get/set for engines.
Like one of the comments said, you should research object prototyping. For a fresh start on JavaScript programming in an object oriented style, read Mozilla's documentation.
Misc.
I'm not convinced having a Ferrari variable and a lotus variable are the best way to accomplish creating cars. I suggest a new object, maybe CarColection or Garage and create/retrieve/store/pimp your cars their. It will allow for later changes, or dynamic modification of the cars.
Purely for fun and for homework: add in the ability to specify exhaust, transmission, tires, driving skill, etc. and then compute horsepower/car level. Pretty soon you could challenge cars up against each other in races! :)


OLEDB table fetching function in C# I'm working on a small project with dBase files from a 3rd party. Realizing that most of the code to fetch tables and form objects is the same, I made this function:

public static IEnumerable<T> OleDbFetch<T>(Func<OleDbDataReader, T> formator, string connectionString, string query)
{
    var conn = new OleDbConnection(connectionString);
    conn.Open();
    var cmd = new OleDbCommand(query, conn);
    var reader = cmd.ExecuteReader();
    while (reader.Read())
        yield return formator(reader);
    conn.Close();
}

That way I can use it with all sorts of classes (pre-made, imposed) that have mostly parameter-less constructors:

class Person
{
    public int ID { get; set; }
    public string Name { get; set; }

    public Person() { }
}

//...

var persons = OleDbFetch<Person>(
    r => new Person()
    {
        ID = Convert.ToInt32(r["ID"]),
        Name = r["Name"].ToString()
    },
    connString, "SELECT ID, Name FROM Person");

Am I missing something obvious, being reckless, or giving myself too much trouble?

Dispose() of types that implement IDisposable with using blocks:

using (var conn = new OleDbConnection(connectionString))
{
    conn.Open();
    using (var cmd = new OleDbCommand(query, conn))
    using (var reader = cmd.ExecuteReader())
    {
        while (reader.Read())
            yield return formator(reader);
    }
}


User registration and login I am doubtful about the security of my PHP code. I am new to programming, but want to learn how to secure things, protect my databases from SQL injection, and other best practices. I'd like to know if my databases are safe from attacks with this code.

public function __construct( $data = array() ) {
               if( isset( $data['username'] ) ) $this->username = stripslashes( strip_tags( $data['username'] ) );
              if( isset( $data['password'] ) ) $this->password = stripslashes( strip_tags( $data['password'] ) );
}

I would appreciate any suggestions to existing scripts if mine is insufficient.
If somebody wants to inspect the entire code of this class, here it is:

<?php

class Users {
public $username = null;
public $password = null;
public $salt = "Zo4rU5Z1YyKJAASY0PT6EUg7BBYdlEhPaNLuxAwU8lqu1ElzHv0Ri7EM6irpx5w";

public function __construct( $data = array() ) {
if( isset( $data['username'] ) ) $this->username = stripslashes( strip_tags( $data['username'] ) );
if( isset( $data['password'] ) ) $this->password = stripslashes( strip_tags( $data['password'] ) );
}

public function storeFormValues( $params ) {
//store the parameters
$this->__construct( $params );
}

public function userLogin() {
$success = false;
try{
$con = new PDO( DB_DSN, DB_USERNAME, DB_PASSWORD );
$con->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION );
$sql = "SELECT * FROM users WHERE username = :username AND password = :password LIMIT 1";

$stmt = $con->prepare( $sql );
$stmt->bindValue( "username", $this->username, PDO::PARAM_STR );
$stmt->bindValue( "password", hash("sha256", $this->password . $this->salt), PDO::PARAM_STR );
$stmt->execute();

$valid = $stmt->fetchColumn();

if( $valid ) {
$success = true;
}

$con = null;
return $success;
}catch (PDOException $e) {
echo $e->getMessage();
return $success;
}
}

public function register() {
$correct = false;
try {
$con = new PDO( DB_DSN, DB_USERNAME, DB_PASSWORD );
$con->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION );
$sql = "INSERT INTO users(username, password) VALUES(:username, :password)";

$stmt = $con->prepare( $sql );
$stmt->bindValue( "username", $this->username, PDO::PARAM_STR );
$stmt->bindValue( "password", hash("sha256", $this->password . $this->salt), PDO::PARAM_STR );
$stmt->execute();
return "Registration Successful
 Login Now";
}catch( PDOException $e ) {
return $e->getMessage();
}
}

}

?>

Let us review!

Why are you strip_slashing and strip_tagging the username and passwords before they enter the database? Why do you care? (You don't, I'll get to that in the end).
Calling __construct() inside of another method of the same object is not recommended. (What does __construct() do? As opposed to what do setUser() and setPassword() do)
Hashing your password:

With your salt constant, it loses its meaning. An attacker with access to your server can see your code, and with access to the database, he can easily run a rainbow table against your password database.
Hashing just with SHA1 once isn't very helpful. SHA1 is too fast and is susceptible to brute force attacks.
The solution to both is to use password_hash() and password_verify() look them up. (Both of those functions require PHP 5.5 or higher. If you don't have PHP 5.5 or higher, you should use the password_compat library by ircmaxell, who is the same person who wrote the above functions for PHP's core.

Indentation - Please, for the love of God.

What's wrong with your sanitizing

Sanitizing is an important job of the server. It makes sure nothing nasty comes in and affects what your users are seeing/experiencing.

Sanitizing is done as late as possible, for a simple reason. When the username is entered to the database, you shouldn't make assumptions on who is going to use that username. It could be your application later on, it could be an API you may or may not choose to provide one day, it could be a desktop application or a mobile application which does not care for HTML formatting.

When you extract that value from the database and are about to put it in HTML, that's the point where you should escape for HTML using htmlspecialchars(). Maybe at a different point you may want to output it to a JSON response for an API, in which case, json_encode is good, but htmlspecialchars() not as much.


Sports quiz in Python I'm a complete beginner at python and I know I'm probably approaching this the wrong way but I've tried making a quiz and can't work out how to make it more concise and work with longer lists of questions and answers without having to use lots of "if" statements. Here's the code:

import random

score = 0

questions = ["In which year did Diego Maradona score a goal with his hand?\n->",
             "In which sport can you win the Davis Cup?\n->",
             "How many minutes is a rugby match?\n->",
             "In which country were the first Olympic Games held?\n->",
             "In which country is the Interlagos F1 circuit?\n->"]
answers = ["1986","tennis","80","greece","brazil"]

print("---------------------------------")
print("You are running Ben's sports quiz")
print("---------------------------------\n")

question = random.sample(range(1,6),5)

for num in question:
    if num == 1:
        user_answer_1 = input(questions[0])
        if user_answer_1.lower() == answers[0]:
            print("Correct\n")
            score = score + 1
        else:
            print("Incorrect\n")
    if num == 2:
        user_answer_2 = input(questions[1])
        if user_answer_2.lower() == answers[1]:
            print("Correct\n")
            score = score + 1
        else:
            print("Incorrect\n")
    if num == 3:
        user_answer_3 = input(questions[2])
        if user_answer_3.lower() == answers[2]:
            print("Correct\n")
            score = score + 1
        else:
            print("Incorrect\n")
    if num == 4:
        user_answer_4 = input(questions[3])
        if user_answer_4.lower() == answers[3]:
            print("Correct\n")
            score = score + 1
        else:
            print("Incorrect\n")
    if num == 5:
        user_answer_5 = input(questions[4])
        if user_answer_5.lower() == answers[4]:
            print("Correct\n")
            score = score + 1
        else:
            print("Incorrect\n")

print("Your final score is",score,"out of 5\n")

input("Press ENTER to exit")

Any help would be greatly appreciated.

It's great that you've put the questions and answers in arrays. You can take further advantage of that in your for loop. Notice how you're always looking at the place in the array that's indexed on less than num? You can use num - 1 as an array index. That eliminates all the duplication in your loop body.

for num in question:
    user_answer = input(questions[num - 1])
    if user_answer.lower() == answers[num - 1]:
        print("Correct\n")
        score += 1
    else:
        print("Incorrect\n")

As mentioned in the comments by Sisoma Munden, score = score + 1 can be turned into score += 1. This works for other arithmetic operators too (someVar *= 3 multiplies someVar by three).

You've also hardcoded the number of questions (in the random.sample line). You can get the length/count of a Python array by passing it to len. For instance, len(questions) is the number of items in the questions array. Therefore, you can replace the random.sample line with this:

question = random.sample(range(1, len(questions) + 1), len(questions))

Then your score-displaying line becomes this (I fixed the display as well, + concatenates strings together, the str function gets the text representation of a number):

print("Your final score is" + str(score) + " out of " + str(len(questions)) + "\n")

Come to think of it, I used len(questions) a lot there. You might consider declaring a variable for that after your arrays:

questionCount = len(questions)

You can then use that in place of len(questions).


The answer to life, the universe, and Project Euler I've never did a real Project Euler in my life, so I figured it was about to do one.
And what better Project Euler to start with than Problem 42?
Project description from Project Euler:

By converting each letter in a word to a number corresponding to its alphabetical position and adding these values we form a word value. For example, the word value for SKY is 19 + 11 + 25 = 55 = t10. If the word value is a triangle number then we shall call the word a triangle word.
Using words.txt, a 16K text file containing nearly two-thousand common English words, how many are triangle words?

I realized quite quickly that to determine if a number is triangular, you can solve a quadratic equation.
The part that I'm the least proud of is how the words are read from the URL. Reading the whole line into memory (the whole text file is essentially one whole line) feels unnecessary, just to then split it into words, cut off the quotation marks, and read it.
Even though I could keep the quotation marks and add a filter in wordValue, it feels like it makes more sense to change the string before passing it there.
I considered using StreamTokenizer to read the words, but that class felt old and ugly and not Java 8-ish at all.
The project euler assignment only asks to print the count of triangular numbers, I figured that it doesn't hurt to also write them and their word values - this can be easily disabled by removing the .peek(System.out::println) lines.
public class ProjEuler42 {

    public static int wordValue(String str) {
        return str.toUpperCase().chars().map(ch -> ch - 'A' + 1).sum();
    }

    public static boolean isTriangular(int number) {
        // The Nth triangular number, T(n), is n*(n+1)/2,
        // by doing T(n) - number = 0 we get a quadratic equation.
        // If the solution to this equation is an integer, it is a triangular number
        double n = Math.round(-0.5 + Math.sqrt(0.25 + 2 * number));
        return 0.5 * n * (n + 1) == number;
    }

    public static void main(String[] args) throws IOException {
        URL url = new URL("https://projecteuler.net/project/resources/p042_words.txt");
        InputStream in = url.openStream();
        try (BufferedReader reader = new BufferedReader(
          new InputStreamReader(in, StandardCharsets.UTF_8))) {
            long count = reader.lines()
                    .flatMap(line -> Arrays.stream(line.split(",")))
                    .map(str -> str.substring(1, str.length() - 1))
                    .filter(str -> isTriangular(wordValue(str)))
                    .peek(System.out::println)
                    .mapToInt(ProjEuler42::wordValue)
                    .peek(System.out::println)
                    .count();
            System.out.println("There are a total of " + count + " triangular words");
        }
    }
}

From a Project Euler perspective, and from a Java 8 perspective, is there anything to improve?

Processing chunk-by-chunk vs line-by-line

Obviously, if the input file was of 1TB size,
containing a single line full of words,
your computer probably wouldn't have enough memory to run this program.
It could be rewritten to process the input chunk by chunk rather than line by line,
and it will work with input of arbitrary size.
It won't be nearly as pretty and definitely not a pleasant reading.

That being said...

The problem description clearly states the input is a 16K text file
If somebody asked you to process a 1TB file with a single line I think you could safely tell them to go to ... Tell them to change their specs.

So, I'd say don't sweat it.
Using Java 8 streams for the effect of elegant code that even non-Java programmers find pleasant to read,
at the expense of reading the entire line 16K into memory,
is a good trade-off.

Another way to cut off the double quotes, without .substring

Instead of this:

.flatMap(line -> Arrays.stream(line.split(",")))
.map(str -> str.substring(1, str.length() - 1))

You could split on sequences of non-word characters,
and skip the first value (an empty string):

.flatMap(line -> Arrays.stream(line.split("\\W+")))
.skip(1)

Slightly more natural arithmetic expression

In isTriangular, instead of multiplying by 0.5,
I find dividing by 2 more natural.
That way it also nicely corresponds to the formula you included there in the comment:

    return n * (n + 1) / 2 == number;

Going beyond the specs a bit

The wordValue method converts input to uppercase,
but the referenced file already contains only uppercase words.
So the step is unnecessary for Euler,
but sure it's more generally useful this way
(except for the hard-coded path to the input file).

Comment out bits not for review

For clarity for reviewers,
I would comment out these lines,
which are clearly meant for yourself only (and for playing around):

        .peek(System.out::println)
        .mapToInt(ProjEuler42::wordValue)
        .peek(System.out::println)


Remove all duplicated elements in a single list I am learning algorithms. And my current algorithm is create a single list, and try to remove all duplicated elements in a sorted list.
Ex:
   given Input: 1 -> 1 -> 1 -> 2 -> 2 -> 3 -> 5 -> 5 -> 7
   Expected Output: 3 -> 7

My bellow solution worked well:

//Definition for singly-linked list.
public class ListNode
{
    public int val;
    public ListNode next;
    public ListNode(int x)
    {
         val = x;
         next = null;
    }
}

public ListNode deleteAllDuplicates(ListNode list)
{
    if (list == null)
    {
        return null;
    }

    var start = list;
    var isStart = true;
    var pre = start;
    var tmp = list.val;

    while (list?.next != null)
    {

        if (tmp == list.next.val)
        {
            while (list?.val == tmp)
            {
                list = list.next;
            }

            if (isStart)
            {
                start = list;
                pre = start;
            }
            else
            {
                pre.next = list;
            }

            if (list == null)
            {
                return start;
            }

            tmp = list.val;
        }
        else
        {
            pre = list;
            list = list.next;
            tmp = list.val;
            isStart = false;
        }
    }

    return start;
}

My question is: What should I do to improve the solution?
Any help are appriciate!

All in all it looks OK to me. I would maybe call the variables something else:

list -> root
tmp -> currentValue
start -> result
etc.

In the ListNode class it's common practice in C# to name public members in CamelCase:

public class ListNode
{
    public int Value;
    public ListNode Next;
    public ListNode(int x)
    {
         val = x;
         next = null;
    }
}

I think that the null-conditional operator is unnecessary in the outer loop.

My own version would look something like:

public static ListNode RemoveDuplicates(ListNode root)
{
  ListNode current = root;
  ListNode result = null;
  ListNode prev = null;

  while (current != null)
  {
    bool singleFound = true;

    while (current?.val == current?.next?.val)
    {
      current = current.next;
      singleFound = false;
    }

    if (singleFound)
    {
      if (result == null)
      {
        result = prev = current;
      }
      else
      {
        prev.next = current;
        prev = prev.next;
      }
    }

    current = current?.next;

    if (singleFound && prev != null)
      prev.next = null;
  }

  return result;
}

Some other considerations:

The name is maybe ambiguous:

My first thought was that it should remove all but one entry per value:

{ 1, 1, 2, 3, 4, 5, 5, 7, 7 } -> { 1, 2, 3, 4, 5, 7 }

Normally I would prefer that a static method/function would leave the argument list untouched, but you actually operate on the nodes in the list directly and return a new ListNode as root of the resulting list. So the original list is in an indefinite state
when the function returns.

My own implementation modified to handle that:

public static ListNode RemoveDuplicates(ListNode root)
{
  ListNode current = root;
  ListNode result = null;
  ListNode prev = null;

  while (current != null)
  {
    bool singleFound = true;

    while (current?.val == current?.next?.val)
    {
      current = current.next;
      singleFound = false;
    }

    if (singleFound)
    {
      if (result == null)
      {
        result = prev = new ListNode(current.val);
      }
      else
      {
        prev.next = new ListNode(current.val);
        prev = prev.next;
      }
    }

    current = current?.next;
  }

  return result;
}

If you had a class like:

public class SortedLinkedList
{
  public void DeleteDuplicates()
  {
    ...
  }
}

I would on the other hand expect DeleteDuplicates() to operate on the current instance.