Small query - removing XML tags I have some code that I wrote almost a year ago exactly (1/9/2013) and I would like to know if I wrote it well or if it can be improved.  I don't have any fun input or output, as these are not set results coming out of this column in the table.

-- =============================================
-- Author: Malachi (Name Changed to Protect the possibly Guilty)
-- Create date: 1/9/2013
-- Description: Will give a list of the Bond Conditions
-- =============================================
CREATE FUNCTION [dbo].[fnBondConditionList]
(
    @BondID int
)
RETURNS Varchar(MAX)
AS
BEGIN
    DECLARE @Result Varchar(MAX)

    SELECT @Result = (Select Justice.dbo.uCode.Description as BondConditions
        FROM Justice.dbo.uCode INNER JOIN
            Justice.dbo.xBondCondition ON Justice.dbo.uCode.CodeID = Justice.dbo.xBondCondition.ConditionID
        WHERE Justice.dbo.xBondCondition.BondID = @BondID
        FOR XML PATH (''))

    SET @Result = replace(@Result, '<BondConditions>','')
    SET @Result = replace(@Result, '</BondConditions>','<br />')
    Set @Result = LEFT(@Result, LEN(@Result) - 1)

RETURN @Result
END

I cannot change the tables.  Here are the table layouts:

uCode:

CREATE TABLE [dbo].[uCode](
    [CodeID] [dbo].[CodeID] NOT NULL,
    [CacheTableID] [dbo].[CacheTableID] NOT NULL,
    [RevisionID] [dbo].[RevisionID] NOT NULL,
    [Code] [dbo].[Word] NOT NULL,
    [RootNodeID] [dbo].[NodeID] NOT NULL,
    [EffectiveDate] [datetime] NULL,
    [ObsoleteDate] [dbo].[ObsoleteDate] NULL,
    [UserIDCreate] [dbo].[UserIDCreate] NOT NULL,
    [TimestampCreate] [dbo].[TimestampCreate] NOT NULL,
    [UserIDChange] [dbo].[UserIDChange] NULL,
    [TimestampChange] [dbo].[TimestampChange] NULL,
    [FilterState] [dbo].[StateCode] NULL,
    [FilterSiteID] [dbo].[GUID] NULL,
    [Description] [dbo].[DescriptionLong] NULL,
CONSTRAINT [PK_uCode] PRIMARY KEY NONCLUSTERED
(
    [CodeID] ASC
)WITH (PAD_INDEX  = OFF, STATISTICS_NORECOMPUTE  = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS  = ON, ALLOW_PAGE_LOCKS  = ON) ON [PRIMARY],
CONSTRAINT [IXuCode1] UNIQUE CLUSTERED
(
    [CacheTableID] ASC,
    [CodeID] ASC
)WITH (PAD_INDEX  = OFF, STATISTICS_NORECOMPUTE  = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS  = ON, ALLOW_PAGE_LOCKS  = ON) ON [PRIMARY]
) ON [PRIMARY]

xBondCondition:

CREATE TABLE [dbo].[xBondCondition](
    [BondID] [dbo].[BondID] NOT NULL,
    [ConditionID] [dbo].[CodeID] NOT NULL,
CONSTRAINT [PK_xBonduCode] PRIMARY KEY CLUSTERED
(
    [BondID] ASC,
    [ConditionID] ASC
)WITH (PAD_INDEX  = OFF, STATISTICS_NORECOMPUTE  = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS  = ON, ALLOW_PAGE_LOCKS  = ON) ON [PRIMARY]
) ON [PRIMARY]

This code was created to stuff everything from one column into a single column of a single record, which is then fed to a stored procedure that fills a datagrid.

Should I be looping this somehow (I get a bad taste in my mouth just typing that)?

I update this because I find myself using the same FOR XML PATH ('') Template for something very similar.

I cannot use CONCAT because I am using SQL SERVER 2008 (wouldn't that be nice).

With all that going into the Stored Procedure, the SPROC Renders something similar to this:

Where the rightmost column is the column of the SPROC that was produced by a function almost exactly the same as above.

If you do not want the XML Tags, then do not add them!

This code frustrates me because:

you have not given any indication as to what this code should be doing.
you are embedding presentation-layer logic in the database layer
you convert all the data to an XML document, and then strip all the XML tags from the result because this appears to be a 'cheap' way to get all the data as a single result instead of multiple rows.
you do not document this
you use less-common features of the database (XML manipulation) without good reason, which locks you in to a single provider for the database ;-)
there is no documented reason for why you strip the last character from the @result... this will strip the closing > from the last
, and will thus leave some invalid X?HTML? I presume? A bug?

As for some nit-picks:

You should not be using SELECT @Result = (...), but should, instead be using SET @Result = (...) See the documentation for SELECT.
you capitalize some of your query key-words, but not all. I recommend using whatever case you do not use for your columns. In your case, your db.schema.table.columns are all lower/CamelCase, so your keywords should all be full CAPITALS. Thus, you should have SELECT everywhere. You have one SELECT, and one Select. You have one LEFT, and two replace. Your consistency of convention should be improved.
you do not use table-aliases for your query objects. Not only do table-aliases make the SQL more readable, but, if you need to change a table name for some reason (e.g. to get data from a view, or different schema), then you have to change multiple places, and could lead to bugs....

As far as I am concerned, this function should be removed entirely, and a simple presentation-layer system should be added that simply appends a
 after each bond description from the query:

SELECT uC.Description
FROM Justice.dbo.uCode uC
INNER JOIN Justice.dbo.xBondCondition xBC
        ON uC.CodeID = xBC.ConditionID
WHERE xBC.BondID = ?


Coupon code generator using the strategy pattern in PHP Below is the implementation in PHP. I aim to have a different coupon code and a different discount returned, based on the type of book (vintage or new).

<?php

interface BookOffer {
    function generateCoupon();
    function generateDiscount();
}

class VintageBookOffer implements BookOffer {
    function generateCoupon() {
        return "VINTAGECOUPONCODE";
    }

    function generateDiscount() {
        return 10.0;
    }
}

class NewBookOffer implements BookOffer {
    function generateCoupon() {
        return "NEWBOOKCOUPONCODE";
    }

    function generateDiscount() {
        return 5.0;
    }
}

class OfferGenerator {

    function generateOffer($bookType) {

        if($bookType == "vintage") {
            $bookObject = new VintageBookOffer();
        }
        else if($bookType == "newbook") {
            $bookObject = new NewBookOffer();
        }

        return $bookObject;
    }
}

$bookType1 = "vintage";
$bookType2 = "newbook";

$offerGenerator = new OfferGenerator();
$bookOffer1 = $offerGenerator->generateOffer($bookType1);
$bookOffer2 = $offerGenerator->generateOffer($bookType2);

echo "You chose book type " . $bookType1 . ". Your coupon code is " . $bookOffer1->generateDiscount() . ", and your discount is " . $bookOffer1->generateCoupon();

echo "You chose book type " . $bookType2 . ". Your coupon code is " . $bookOffer2->generateDiscount() . ", and your discount is " . $bookOffer2->generateCoupon();

?>

Does this look correct? The only reason I think it might be incorrect is because it does not use type hinting, which is frequently seen in the Strategy Pattern.

It's correct. But if you want to restrict return types to something specific (and I would, because db) you could write interface functions like this:

function generateCoupon(): string;
function generateDiscount(): int;

http://php.net/manual/en/functions.arguments.php#functions.arguments.type-declaration

http://php.net/manual/en/functions.returning-values.php


Find words that can be typed using only one row on a keyboard on leetcode.com, there is a question with the following description:

  Given a List of words, return the words that can be typed using letters of alphabet on only one row's of American keyboard like the image below.
  link to the question here.

Here is my implementation:

class Solution {
public:
    std::vector<std::string> findWords(std::vector<std::string>& words)
    {
        std::unordered_set<char> firstRow = {'Q', 'W', 'E', 'R', 'T', 'Y', 'U', 'I', 'O', 'P'};
        std::unordered_set<char> secondRow = {'A','S','D','F','G','H','J','K','L'};
        std::unordered_set<char> thirdRow = {'Z','X','C','V','B','N','M'};
        std::vector<std::string> result;

        size_t vecSize = words.size();
        for(size_t i = 0; i < vecSize; ++i)
        {
                if(sameRow(words[i], firstRow))
                    result.push_back(words[i]);
                else if(sameRow(words[i], secondRow))
                    result.push_back(words[i]);
                else if(sameRow(words[i], thirdRow))
                    result.push_back(words[i]);
        }
        return result;
    }
    bool sameRow(const std::string& words, const std::unordered_set<char>& row)
    {
                size_t wordSize = words.size();
                for(size_t j = 0; j < wordSize; ++j)
                {
                    if(row.find(toupper(words[j])) == row.end())
                    {
                       return false;
                    }
                }
        return true;
    }
};

Algorithm/Data Structure Selection

Right now you're using three separate sets, one of the letters in each row of the keyboard.

Instead, I think I'd use a single map from character to row number. Then you can start with the first character in the string, and find what row it's on. Then you check whether the other characters in the word are in the same row. Right now, you may look up each character in a string up to three separate times. Changing the algorithm, you can reduce that to (at most) one lookup per character.

Since you have a dense set of inputs for the map (i.e., only ASCII letters) I'd probably also use an array/vector for the mapping, instead of an unordered map. A vector/array will avoid hashing, and it's a dense structure, so you get really good cache utilization.

Avoid Unnecessary Transformations

It's probably simpler to create a mapping for all possible input characters instead of using toupper on the input, then looking it up in the set/map.

Consider other Looping Constructs

Instead of a loop like this:

            size_t wordSize = words.size();
            for(size_t j = 0; j < wordSize; ++j)
            {
                if(row.find(toupper(words[j])) == row.end())
                {
                   return false;
                }
            }

I'd probably use a loop more like this:

for (auto c : words[j] )
    if (row.find(c) == row.end())
        return false;
return true;

...or you could use a standard algorithm instead:

return std::all_of(words[j].begin(), words[j].end(),
    [&](char c) { return row.find(c) != row.end(); });

(Though, as outlined above, I'd rather change it to look up the row number instead).


Templated CSV file parser I want to parse a CSV file and store the data using the istream >> operator but I am running into big performance issues.

Here is the current version:

#include <vector>
#include <sstream>
#include <fstream>
#include <string>
#include <iostream>

template <class T>
class   CSV
{

    std::vector<std::vector<T>> _data;
    size_t _width;
    size_t _height;

public:

    CSV(const std::string& filepath)
    {
        std::ifstream file(filepath);
        if (!file)
            throw std::exception();

        for (std::string line; std::getline(file, line);)
        {
            std::stringstream ss1(line);
            std::vector<T> row;

            for (std::string field; std::getline(ss1, field, ',');)
            {
                std::stringstream ss2(field);
                T item;

                ss2 >> item;
                row.push_back(item);
            }
            _data.push_back(row);
        }
        _height = _data.size();
        if (!_height)
            throw std::exception();
        _width = _data[0].size();

        for (auto& row : _data)
            if (row.size() != _width)
                throw std::exception();
    }

    size_t  Width(void)
    {
        return _width;
    }
    size_t  Height(void)
    {
        return _height;
    }

    const std::vector<std::vector<T>>   &Data(void)
    {
        return _data;
    }
};

It works but it's slow because I start by making a copy of each line in the file. I then make a copy of that line inside of a stringstream. I then parse each field in that line and make a copy of it. I then make a copy of that inside another stringstream, and finally I extract the data from the stringstream.

Basically there is too much copying, but I honestly don't know how to avoid it with stringstreams. I know there are other ways to parse things that are not stringstreams but then I miss out on using the >> operator and the point of making a templated CSV parser.

For an idea of its speed I have an 80MB file and it takes 11 seconds to parse using CSV<int>. I also have -03 turned on and am using clang++.

#include <iostream>

You don't seem to need this include.

size_t _width;
size_t _height;

These should both be std::size_t.

CSV(const std::string& filepath)

Single argument constructors should almost always be explicit.

if (!file)
    throw std::exception();

What you want to throw here is std::runtime_error or something derived from it, not std::exception.

for (std::string line; std::getline(file, line);)
{
    std::stringstream ss1(line);
    std::vector<T> row;

    for (std::string field; std::getline(ss1, field, ',');)
    {
        std::stringstream ss2(field);
        T item;

        ss2 >> item;
        row.push_back(item);
    }
    _data.push_back(row);
}

Alright, let's get into the meat of this.

for (std::string line; std::getline(file, line);)

There's really nothing you can do to improve the outer for loop prologue. One would hope line's memory would get reused, so there will be allocations only until the longest line is read. You could pull line out of the for prologue and reserve() a reasonable size for it to potentially eliminate all allocations in the loop, but then you'd have to know in advance how much to allocate, which doesn't seem generally likely.

std::stringstream ss1(line);

Here is where you can start making gains. The purpose of ss1 is just to split line by commas. That doesn't seem necessary. You can drop ss1, and change the inner for loop to:

std::string field;
for (auto p = line.cbegin(); get_field(p, line.cend(), field); )

Then you could write a get_field() function (maybe private and static in the class) something like:

template <typename ForwardIterator>
auto get_field(ForwardIterator& next, ForwardIterator last, std::string& field) -> bool
{
    if (next == last)
        return false;

    if (*next == ',')
        ++next;

    // This just gets everything up to the next comma, as your
    // current code does... but you could modify it to handle
    // escaped or quoted values.
    auto const first = next;
    next = std::find(next, last, ',');
    field.assign(first, next);

    return true;
}

Taking field (and first) by non-const reference is generally bad practice, but for an internal implementation function, it's okay - especially given that you're gunning for speed gains.

On top of this, you can move field outside of even the outer for loop - again, generally bad practice, but you're gunning for speed not beauty.

Here's another optimization opportunity. Currently you don't bother to check for row size until after everything's read. Instead, what you can do is start with _width as zero, and then at the end of the loop, set it to row.size(). Then right after you create row, you can do row.reserve(_width);. That will (hopefully) prevent the push_back()s in the inner loop from triggering allocations.

Now the inner loop is:

{
    std::stringstream ss2(field);
    T item;

    ss2 >> item;
    row.push_back(item);
}

There's not much you can do to improve this for efficiency, except that the last line should be row.push_back(std::move(item));. What you should do, though, is check that the formatting worked.

And of course, after the loop, you should move the row into _data, not copy it.

So the modified loop might look something like:

std::string field; // outside the loop so we can continually reuse the memory
for (std::string line; std::getline(file, line);)
{
    std::vector<T> row;
    row.reserve(_width); // reserve what we think the width is going to be, to prevent allocations when doing push_back()

    for (auto p = line.cbegin(); get_field(p, line.cend(), field);)
    {
        std::stringstream ss{field};

        T item;
        if (!(ss >> item))
            throw std::runtime_error{"invalid field"};

        row.push_back(std::move(item));
    }

    _width = std::max(_width, row.size()); // get a better estimate of the width
    // Note that you could actually do a check here to make sure the
    // width stays consistent, and fail early if not rather than
    // waiting until the whole file has been read before failing.

    _data.push_back(std::move(row));
}

That is about the best I can think of, performance-wise, for the main loop.

if (!_height)
    throw std::exception();

Again, you should throw std::runtime_error, not std::exception. std::exception should only be used as base class.

_width = _data[0].size();

If you get the width in the loop as I suggested, you don't need this line.

for (auto& row : _data)
    if (row.size() != _width)
        throw std::exception();

This could be done with an algorithm:

if (!std::all(_data.cbegin(), _data.cend(), [_width](auto&& row) { return row.size() == _width; }))
    throw std::runtime_error{"different widths"};

On to the other functions....

size_t  Width(void)
{
    return _width;
}

You don't need to declare void to specify an empty argument list; that's archaic C stuff.

This function, and Height() as well, can be both const and noexcept, and should be at least const.

const std::vector<std::vector<T>>   &Data(void)
{
    return _data;
}

It is standard practice in C++ to keep pointer and reference modifiers with the type. In other words const std::vector<std::vector<T>>& Data(), not const std::vector<std::vector<T>>   &Data().

This function should also be both const and noexcept.

Summary

Try to use as much information from previous iterators to make subsequent iterators more efficient. For example, once you know the width of a row, use that to preallocate the row vector.
Moving allocating variables (like strings) that get reused repeatedly out of the loop may allow you to reuse the memory, and avoid allocations.
Move whenever possible, rather than copying.
You don't need to create a string stream just to split on commas.
std::exception is a base class, not a general use exception class. Use std::logic_error or std::runtime_error or derived classes as appropriate.


slide out navigation bar seems too simple but it works, is it? http://crystalcleanhomes.com/slide/

There are countless scripts that perform this simple slide-out navigation menu but they are all so bloated and the styles are so convoluted that I decided to make my own and it seems almost too simple and too basic so I've come here for criticism. From my testing it even works in IE7 and an older Android tablet (and of course flawlessly in Chrome, iOS, FF, etc.). So please critique this!

Here is a fiddle to more easily see the code: http://jsfiddle.net/9nT7B/

Question: does this meet compliance?

$(function(){
    $('#menubutton').click(function(e){
        e.preventDefault();
        e.stopPropagation();
        });
    function handler1(){
        $("#page").animate({left:"200px"},150);
        $("#menu").css("overflow-y","auto");
        $("#menubutton").one("click",handler2);
        }
    function handler2(){
        $("#page").animate({left:"0"},150);
        $("#menu").css("overflow-y","hidden");
        $("#menubutton").one("click",handler1);
        }
    $("#menubutton").one("click",handler1);
    $("#page").click(handler2);
    $("#menu").css("visibility","visible");
    });

From a once over:

You should put the closing brace at the level of $ for readability:

$(function () {
    $('#menubutton').click(function (e) {
        e.preventDefault();
        e.stopPropagation();
    });

    function handler1() {
        $("#page").animate({
            left: "200px"
        }, 150);
        $("#menu").css("overflow-y", "auto");
        $("#menubutton").one("click", handler2);
    }

    function handler2() {
        $("#page").animate({
            left: "0"
        }, 150);
        $("#menu").css("overflow-y", "hidden");
        $("#menubutton").one("click", handler1);
    }
    $("#menubutton").one("click", handler1);
    $("#page").click(handler2);
    $("#menu").css("visibility", "visible");
});

handler1, handler2 are unfortunate names, these function names should convey what the function does ( perhaps slideRight and slideLeft ? )
You access $('#menubutton') a number of times, you should cache it with var $menuButton = $('#menubutton') and then access $menuButton, you can use the same technique for the other jQuery calls as wel
You are repeating yourself between handler1 and handler2, but it seems that trying to find a common routine would make this code more complex, so I would leave that alone


Pig Latin Application in rust Just finished an exercise in the Rust online book and I wanted to know if there is anything worth talking about in the code I wrote... if there's any mistake or optimization possible.
Convert strings to pig latin

The first consonant of each word is moved to the end of the word and
“ay” is added, so “first” becomes “irst-fay”.

Words that start with
a vowel have “hay” added to the end instead (“apple” becomes
“apple-hay”).

Keep in mind the details about UTF-8 encoding!

use std::io;
fn main() {
    let mut user_input = String::new();
    io::stdin()
        .read_line(&mut user_input)
        .expect("Failed to read line.");
    for word in user_input.split_whitespace() {
        match word.chars().nth(0).unwrap() {
            'a' | 'e' | 'i' | 'o' | 'u' | 'y' => print!("{} ", format!("{}-hay", word.trim())),
            _ => print!("{} ", format!("{}{}-ay", &word[word.chars().next().unwrap().len_utf8()..].trim(), word.chars().nth(0).unwrap())),
        };
    }
    println!();
}

Some small remarks:

according to the documentation, you want to output irst-fay on first, but your code will output irstf-ay instead

your code will only work on lower case words, but not on "Apple"

instead of print!("{} ", format!(...)) use print!(...)

instead of nth(0) use next()

I recommend you to format your code with rustfmt

instead of &word[word.chars().next().unwrap().len_utf8()..] you can use c.len_utf8() if you name your match instead of using _, e.g.
match word.chars().nth(0).unwrap() {
     'a' | 'e' | 'i' | 'o' | 'u' | 'y' => print!("{}-hay ", word.trim())),
      c => print!("{}-{}ay ", &word[c.len_utf8()..].trim(), c),
};

if you know that your index will end up at a proper UTF8 boundary you can use &word[1..] and &word[..1] instead of your indexing, but your variant is a lot safer. Well done here, as invalid string slicing is a common error in first Rust experiences :)


Filtering stocks by dividend yield I have a dict of stock tickers (KEY) and a data point (VALUE; e.g. dividend yield).

I want to know which tickers have a value between a specified min and max value.

Given

data = {'KO': 1, 'WMT': 0, 'IBM': 3, 'PG': 2}
gte = 1
lte = 2

Func1

def func1(data, gte=None, lte=None):
    data_set = set(data)
    gte_set = set(filter(lambda k: data[k] >= gte, data)) if gte else data_set
    lte_set = set(filter(lambda k: data[k] <= lte, data)) if lte else data_set
    return data_set.intersection(gte_set, lte_set)

Func2

import operator
def func2(data, gte=None, lte=None):
    if gte is None and lte is None:
        return data
    #without dict data gets modified
    _data = dict(data)
    for k, v in data.items():
        if gte and operator.lt(v, gte):
                del _data[k]
        if lte and operator.gt(v, lte):
                del _data[k]
    return set(_data)

Func3

def func3(
    data,
    gte=float('-inf'),
    lte=float('inf')
):
    return set(filter(
        lambda k: gte <= data[k] <= lte,
        data
    ))

Example

func1(data, gte=gte, lte=lte) == {'PG', 'KO'}
func2(data, gte=gte, lte=lte) == {'PG', 'KO'}
func3(data, gte=gte, lte=lte) == {'PG', 'KO'}

First of all, I would rename the bounds to be more meaningful, from gte to min_value, and from lte to max_value.

Function 1 comes close to being a good idea, but could be implemented more smartly.  The special treatment of any falsy value (notably 0) to skip filtering is probably a bad idea.  The set intersection operation should be avoidable.

def func1_improved(data, min_value=None, max_value=None):
    if min_value is not None:
        data = {(k, v) for k, v in data if v >= min_value}
    if max_value is not None:
        data = {(k, v) for k, v in data if v <= max_value}
    return data.keys()

Function 2 is not equivalent to the other two: it returns a dictionary rather than a key set.  It suffers from the problem that it sometimes returns the original dictionary, and sometimes returns a copy.  It also treats 0 as a special parameter value.

def func2_improved(data, min_value=None, max_value=None):
    data = dict(data)    # Make a copy
    for k, v in data.items():
        if min_value is not None and v <= min_value:
                del data[k]
        if max_value is not None and v >= max_value:
                del data[k]
    return data.keys()

Function 3 is quite good, especially when formatted as a one-liner using a set comprehension:

def func3_improved(data, min_value=float('-inf'), max_value=float('inf')):
    return set(k for (k, v) in data.items() if min_value <= v <= max_value)

That is my recommendation.  You could eliminate some comparisons by making special cases for when min_value or max_value is missing, but I'd prefer not to ruin the elegance of the one-line solution.


Use of macros to aid visual parsing of SFINAE template metaprogramming I've recently been introduced to SFINAE to solve the problem of unwanted promotion precedence.

i.e. I was hoping to catch integer types with Foo::Foo(long) and floating-point types with Foo::Foo(double), but alas int -> double rather than long.

    /*
    Constructors for various types, for example, 'Object{"foo"}, Object{42}, Object{3.14} should create a String Long Float respectively

    There is a problem with constructors.

    Python supports a single integer type, which we wrap with the Long class
    And the single floatingpoint type, which we wrap with the Float class

    In an ideal world we would just provide two overloads to allow a generic
    Object to be initialised as one of these types:

        Object(long   l) : Object{ Long {l} }  { }
        Object(double d) : Object{ Float{d} }  { }

    We want Object{5} to create a Long{5}
    Unfortunately, int gets promoted to double, not long ( http://stackoverflow.com/a/27276398/435129 )

    Very annoying. Fortunately we can use some SFINAE cunning.
    */
    #define DECAY(T) \
                            typename std::decay<T>::type

    #define IS_INTEGRAL(T) \
                            std::is_integral< DECAY(T) >::value

    #define IS_FLOATING(T) \
                            std::is_floating_point< DECAY(T) >::value

    #define SUBFAIL_UNLESS(PRED) \
                            typename X = typename std::enable_if<PRED>::type

    /*
    Note that the first template encounters a substitution failure for any non-integral type,
    hence only redirects integral types to init(long(t))

    Similarly for floatingpoint types.

    Note also the unfortunate use of an empty '...' C parameter expansion.
    Without this the compiler will complain that two templates are attempting
    to wrap a function with the same signature.

    Unfortunately it isn't smart enough to know that the conditions are mutually exclusive.

    If you required trapping of three or more such special cases, see NOTE_3_CASES at the bottom of the file.
    Also http://ideone.com/oDOSLH
    */

public:
    template<typename T, SUBFAIL_UNLESS(IS_INTEGRAL(T)) > explicit Object( T&& t      ) { init(long  (t)); }
    template<typename T, SUBFAIL_UNLESS(IS_FLOATING(T)) > explicit Object( T&& t, ... ) { init(double(t)); }

private:
    void init(long);
    void init(double);

    #undef DECAY
    #undef IS_INTEGRAL
    #undef IS_FLOATING

I know that macros are generally frowned upon. However, in this case they provide a very clear solution, making it very easy for my brain to parse the code.

I would like to know whether in this case the use of macros is justified, and if not what the best alternative would be.

Rule #1 of code formatting: Write readable code.

Rule #2: Don't do anything people tell you not to do... until you know better.

The rules against macros are wise because macros can often be very unintuitive.  However, as you have noticed, there are situations where they are very helpful.  The problem with macros is that they appear to be a panacea for formatting, and it is not until  much later that you realize there were serious fundamental issues.

That being said, they're in the language.  They have uses.  So if you take the time to understand them, then you can start to use them wisely.  The two major issues for macros are:

They are global symbols, causing surprise replacements in the most unlikely of places
It is easy to write a macro that looks like it does what you want, when it actually compiles into something else.

Global symbols

Nothing is worse than having someone else's symbols screw with your code.  If someone defines a macro, and it conflicts with your code, the error is almost always unreadable.  Consider the most nefarious one, #define max (a, b) ((a)>(b) ? (a) : (b)), which is defined as part of windows.h.  If you've been writing Linux apps, you'll find this suddenly breaks when you had a local named "max."

By your choice to #undef the symbols at the end, it is clear you are aware of this.  You did a good job of making sure your defines don't hurt someone else.  However, you are still at the whim of anyone who uses the same names as you carelessly.  So good job with being careful, but you're not out of the woods.

** Surprise compilations **

The number on reason why people dislike macros is that they often do things you did not expect.  For example, in the max example, a can get evaluated twice.  Also, macros and commas don't play well.  Macros are generally unaware of brackets, so things like max(getValue(1, 2), getValue(3, 4)) can surprise you with errors.  Your code doesn't have any of these, but always be aware of the costs when you're trying to bend code style rules.

However, you do have a few interesting tidbits.  For example, did you know that you shouldn't say std::enable_if in a macro?  That will do a context-specific search for a namespace named "std."  In the wrong situations, that could cause problems.  The correct phrasing is ::std::enable_if, which forces it to look in the global scope for the right std.

Think of others

So, in you cases, the macros work.  You generally thought it through.  Now lets take this code into a business scenario.  Developers with much less macro practice than you are going to work on this code.  They are going to develop stylistic habits to take parts of how you write.  Do you really get enough readability here to warrant potentially confusing code down the line?

Options

So what else can you do?  Since macro definitions are usually treated as "hard to read," I feel no qualms writing a few struct to help out.  By using structs, I get to avoid the global issues of macros, and they always compile the way you expect them to.

Consider using a template template argument like this:

template <template <typename> Pred, T>
struct subfail_unless
: std::enable_if<Pred<typename std::decay<T>::type> >
{ };

template<typename T,
         typename subfail_unless<std::is_integral, T>::value >
explicit Object( T&& t      ) { init(long  (t)); }

template<typename T,
         typename subfail_unless<std::is_floating_point, T>::value >
explicit Object( T&& t, ... ) { init(double(t)); }

Why I like this:

If you are writing code like this, you MUST know SFINAE, so you aren't going to be threatened by the templating.
It you are are just reading this, you don't need to know why it works, you just need to recognize the words that matter.

Consider how few extra characters this has.

macros:              SUBFAIL_UNLESS(     IS_FLOATING       (T))
templates:  typename subfail_unless<std::is_floating_point, T>::value
---------------------------------------------------------------------
difference: typename                std::                     ::value

This code can now be reused in a header, instead of needing to #define it everywhere you intend to use it.  It is now using only namespaced structs, so it is as safe as any other code.


Project Euler 22: Names scores alternative Can someone tell me how to improve my following python code.

Code:

a = open('/home/Username/1st few programms/names.txt')

for y in a:
    c = y.split(",")
    c.sort()

dict = {'"':0, "A": 1, "B": 2,"C": 3,"D": 4, "E": 5,"F": 6,"G": 7,"H": 8,"I": 9,"J": 10,"K": 11,"L": 12,"M": 13,"N": 14,"O": 15,"P": 16,"Q": 17,"R": 18, "S": 19, "T": 20, "U": 21, "V": 22,"W": 23,"X": 24, "Y": 25, "Z": 26}

sum = 0
for x in range(len(c)):
    localsum = 0
    for y in c[x]:
        localsum += dict[y]
    sum += localsum*(x+1)
print sum

Problem statement:

  Using names.txt, a 46K text file containing over five-thousand first
  names, begin by sorting it into alphabetical order. Then working out
  the alphabetical value for each name, multiply this value by its
  alphabetical position in the list to obtain a name score.

  For example, when the list is sorted into alphabetical order, COLIN,
  which is worth 3 + 15 + 12 + 9 + 14 = 53, is the 938th name in the
  list. So, COLIN would obtain a score of 938 × 53 = 49714.

  What is the total of all the name scores in the file?

a, dict, c and sum are all problematic names, for two different reasons. Single character variable names should be used sparingly, really only in brief contexts where the intent is quite clear like for i in range(10) where it's obvious that i is an integer and follows for loop conventions. for y in a is very unclear because you haven't made it obvious what y is or should be, and a would be much harder to follow if not for the fact that it's declared just above. Even so the choice of letters seems totally arbitrary. I'd replace a with f for file, since it's localised.

The other naming issue is that using the names dict and sum are actually overwriting builtin functions and should be avoided, especially as I'm going to suggest that you use sum, which you can't do if it's been overwritten.

Also instead of using a plain open, you should use with open(). That's Python's context manager which handles file closing for you. It ensures that a file is always closed correctly, even if an error arises or you don't call file.close(). Note, you didn't call a.close() so clearly this is useful:

with open('/home/Username/1st few programms/names.txt') as f:
    for line in f:
        names = line.split(",")
        names.sort()

Next, this code you're using makes no sense. You're iterating over each line in the file but just creating a new list of names each time. If all the names are stored on one line then this works but is redundant, and I'd instead suggest this:

with open('/home/Username/1st few programms/names.txt') as f:
    names = f.read().split(",")
names.sort()

read() will just read the full string of the file in so that then split can separate it based on commas. If you do have names on multiple lines, then you want the loop but you want to add more values to names each time:

names = []
with open('/home/Username/1st few programms/names.txt') as f:
    for line in f:
        names += line.split(",")
names.sort()

The algorithm that determines a person's score is quite unusual, so it would actually be best to create a function for it. One that takes a name and position in the list and returns their score. It can do largely what your inner loop does:

def name_score(index, name):
    localsum = 0
    for character in name:
        localsum += char_points[character]
    return localsum * (index + 1)

However, this is one of the places I'm going to suggest you use sum. You can pass a generator expression into sum to do this calculation efficiently in one line. A generator expression is a for loop like expression, here's how it'd look for this function:

def name_score(index, name):
    return sum(char_points[character] for character in name) * (index + 1)

You can see how there's the same for _ in syntax as before, just that now it's collapsed into one line. You no longer need to have a localsum and sum is faster than using a for loop.

Now with this function, the main loop is simplified to:

sum = 0
for i in range(len(names)):
    sum += name_score(i, names[i])
print sum

First though, let's get rid of names[i] as that's ugly. Use Python's enumerate function that allows you to both get the indices and values of a list together, like this:

for i, name in enumerate(names):
    sum += name_score(i, name)

This is much cleaner now, but we can also use sum in this case to remove your whole loop, in fact it can skip straight to printing the sum:

print sum(name_score(i, name) for i, name in enumerate(names))

Again, this is both more readable and much faster. This also means that you no longer have a sum value that needs to be renamed.


Finding next greater element This is the Daily Temperatures problem at leetcode.com:

  Given a list of daily temperatures, produce a list that, for each day
  in the input, tells you how many days you would have to wait until a
  warmer temperature. If there is no future day for which this is
  possible, put 0 instead.

  For example, given the list temperatures = [73, 74, 75, 71, 69, 72,
  76, 73], your output should be [1, 1, 4, 2, 1, 1, 0, 0].

  Note: The length of temperatures will be in the range [1, 30000]. Each
  temperature will be an integer in the range [30, 100].

This code works for smaller inputs and fails for list of length 10,000. What would you recommend me to change or improve in this code?

class Solution(object):
    def dailyTemperatures(self, temperatures):
        """
        :type temperatures: List[int]
        :rtype: List[int]
        """
        #O(n2)
        #max(temperatures)
        res=[]
        for i in range(len(temperatures)):
            cnt=0
            for j in range(i+1,len(temperatures)):
                cnt+=1
                if temperatures[i]<temperatures[j]:
                    res.append(cnt)
                    cnt=0
                    break
                elif j==len(temperatures)-1:
                    res.append(0)
                    cnt=0
                    break
        res.append(0)
        return res

Start by setting your res list with \$ 0 \$ values. Thus:

res = [0] * len(temperatures)

You won't need the elif condition now. Nor would you need the ending res.append. All res.append inside the loop will be replaced by res[i] = counter values.

Use a more descriptive variable naming than skipped vowels (counter instead of cnt, result instead of res etc.)

You compute len(temperatures) a total of \$ n^2 \$ number of times. Keep the value inside another variable instead.

As for the complexity, your current algorithm is \$ O(n^2) \$, which might be reduced by using a more efficient algorithm. Take a look at this SO answer making use of a LIFO queue (stack) and getting the results in \$ O(n) \$.


List all classes in a package directory I have a package directory pkg with several classes that I would like to build into a convenient dict property.

The structure of pkg/ looks like:

pkg/base.py:

class _MyBase(object):
    pass

pkg/foo.py:

from .base import _MyBase

class Foo(_MyBase):
     pass

And in pkg/__init__.py, it is a bit clunky, but once pkg is imported, a all_my_base_classes dict is built with a key of the class name, and value of the class object. The classes are all subclasses of pkg.base._MyBase.

import os
import sys
import pkgutil

import base
# I don't want to import foo, bar, or whatever other file is in pkg/

all_my_base_classes = {}
pkg_dir = os.path.dirname(__file__)
for (module_loader, name, ispkg) in pkgutil.iter_modules([pkg_dir]):
    exec('import ' + name)
    pkg_name = __name__ + '.' + name
    obj = sys.modules[pkg_name]
    for dir_name in dir(obj):
        if dir_name.startswith('_'):
            continue
        dir_obj = getattr(obj, dir_name)
        if issubclass(dir_obj, base._MyBase):
            all_my_base_classes[dir_name] = dir_obj

Running it from an interactive Python shell, one directory below pkg/:

>>> import pkg
>>> pkg.all_my_base_classes
{'Foo': <class 'pkg.foo.Foo'>}

So it works as expected, but pkg/__init__.py is pretty terrible looking. How can it be better?

Since the classes are all subclasses of _MyBase, they can be accessed via _MyBase.__subclasses__() after they have been imported:

for (module_loader, name, ispkg) in pkgutil.iter_modules([pkg_dir]):
    importlib.import_module('.' + name, __package__)

all_my_base_classes = {cls.__name__: cls for cls in base._MyBase.__subclasses__()}

For importing the modules, I followed the advice of Nihathrael.


How to know when I need to use get/set and when not? I have read several articles using Google, but I somehow do not really get what they do, and when it is needed. Below I'll give 2 pieces of code, in one using get/set and in the other without, and they both work (at least in this example).

Example 1

public class Example
{
    public int number;
}

class Program
{
    static void Main(string[] args)
    {
        Example example = new Example();

        example.number = 5;

        Console.WriteLine(example.number);
        Console.ReadKey();
    }
}

Example 2

public class Example
{
    private int number;
    public int Number
    {
        get { return number; }
        set { number = value; }
    }
}

class Program
{
    static void Main(string[] args)
    {
        Example example = new Example();

        example.Number = 5;

        Console.WriteLine(example.Number);
        Console.ReadKey();
    }
}

I've added a piece of real code below so it may be improved and perhaps shown examples on these accessors.

// Main
public class Asteroid
{
    Random random = new Random();

    public Texture2D texture;
    public Vector2 position, origin;
    public float rotationAngle;
    public int speed;
    public Rectangle boundingBox;
    public bool isVisible;
    public float randomX, randomY;

    // Constructor
    public Asteroid(Texture2D newTexture, Vector2 newPosition)
    {
        position = newPosition;
        texture = newTexture;
        speed = 4;
        randomX = random.Next(0, 750);
        randomY = random.Next(-600, -50);
        isVisible = true;
    }

    // Load Content
    public void LoadContent(ContentManager Content)
    {

    }

    // Draw
    public void Draw(SpriteBatch spriteBatch)
    {
        if (isVisible)
            // spriteBatch.Draw(texture, position, Color.White);
            spriteBatch.Draw(texture, position, null, Color.White, rotationAngle, origin, 1.0f, SpriteEffects.None, 0f);
    }

    // Update
    public void Update(GameTime gameTime)
    {
        // Set bounding box for collision
        boundingBox = new Rectangle((int)position.X - (texture.Width / 2), (int)position.Y - (texture.Height / 2), texture.Width, texture.Height);

        origin.X = texture.Width / 2;
        origin.Y = texture.Height / 2;

        // Update Movement
        position.Y = position.Y + speed;

        if (position.Y >= 950)
        {
            isVisible = false;
        }

        // Rotating Asteroid
        float elapsed = (float)gameTime.ElapsedGameTime.TotalSeconds;
        rotationAngle += elapsed;
        float circle = MathHelper.Pi * 2;
        rotationAngle = rotationAngle % circle;
    }
}

First of all, I'd make the texture and position fields private. They're set from the outside, and the outside doesn't need to change them.

Then I'd extend that. Anything your class needs to be given, add to the constructor. If Asteroid makes it itself, make it private. If something needs to look at it, make a get only property to access it. If it needs to be modified, make it an autoproperty.

The only thing that seems like a good candidate for a property to me is your Origin. Something may want to sort Asteroids by Origin.

If, following my advice, you reach more than 4 parameters, you should make a new class to either use as the parameter or deal with those values in the Asteroid.

If your teachers have been telling you to make everything a public field, ignore them. They're wrong. Maybe follow their advice on assignments for the sake of credits, but never in production code.


Find big files on the harddrive For practicing Python I wrote a script which scans the harddrive for large files and lists them in a report. The script takes the root path and then all the subfolders are scanned aswell. Also the minimum size for the files needs to be defined. Smaller files are not listed.

The code:

find_big_files.py

"""
Find big files on the harddrive and writes a report were the big files
are located
Supply root folder and desired size to find the files.
"""

import os

def size_with_biggest_unit(size: int) -> str:
    """
    Turns size in Bytes into KB, MB, GB or TB
    """
    unit: str = ""

    if size > (1024 ** 4):
        unit = "TB"
        size = int(round(size / 1024 ** 4))
    elif size > (1024 ** 3):
        unit = "GB"
        size = int(round(size / 1024 ** 3))
    elif size > (1024 ** 2):
        unit = "MB"
        size = int(round(size / 1024 ** 2))
    elif size > 1024 ** 1:
        unit = "KB"
        size = int(round(size / 1024))

    return str(size) + unit

def size_in_bytes(size: str) -> int:
    """
    Turns size in KB, MB, GB or TB into Byte
    """
    ret_size: int = 0
    if size.endswith("TB"):
        ret_size = int(size.strip("TB")) * (1024 ** 4)
    elif size.endswith("GB"):
        ret_size = int(size.strip("GB")) * (1024 ** 3)
    elif size.endswith("MB"):
        ret_size = int(size.strip("MB")) * (1024 ** 2)
    elif size.endswith("KB"):
        ret_size = int(size.strip("KB")) * 1024
    elif size.isdigit():
        ret_size = int(size)
    else:
        raise Exception(("Input size should be digit + TB/GB/MB/KB or digit."
                         "Size was: ") + size)
    return ret_size

def sort_by_size(big_files: list) -> list:
    """
    Sort dictionary with folder_name, filename and size by
    filesize in decreasind order
    """
    return sorted(big_files, key=lambda k: k['file_size'], reverse=True)

def write_report_of_big_files(big_files: list):
    """
    Write report in same folder as the script is excecuted.
    Report contains formated output of found big files
    """
    with open('big_files_report.txt', 'w') as file:
        for big_file  in big_files:
            file.write(size_with_biggest_unit(big_file['file_size'])
                       + '\t'
                       + big_file['filename']
                       + '\t'
                       + big_file['folder_name']
                       + '\n')

def find_big_files(root_folder: str, input_size: str):
    """
    Checks from all files in root and sub folder if they are exceeding
    a certain size
    """
    size = size_in_bytes(input_size)
    big_files: list = []

    for folder_name, subfolders, filenames in os.walk(root_folder):
        for filename in filenames:
            file_size = os.path.getsize(folder_name + '\\' + filename)

            if file_size > size:
                big_file: dict = {'folder_name': folder_name,
                                  'filename': filename,
                                  'file_size': file_size}
                big_files.append(big_file)

    sorted_big_files = sort_by_size(big_files)
    write_report_of_big_files(sorted_big_files)

find_big_files("E:\\", "100MB")

I checked the code with PyLint and MyPy.

PyLint still gives one warning in line 86 (in find_big_files ):

  Unused variable 'subfolders' [W:unused-variable]

It is true I dont use the variable subfolders but I need to supply three variables in the for loop to use folder_name and filename which i get from os.walk(root_folder) or not?

Also I wonder if I used Type Annotations correctly. I used them here the first time and already found some bugs with MyPy (which are already fixed in the posted code).

Other than that:

Are there any other smells?

Is the code easy to follow?

Can anything be done easier?

Feel free to comment on anything suspicious you can find.

size_with_biggest_unit should be streamlined. Move the unit names into a list:

unit_names = [ "", "KB", "MB", "TB" ]

and iterate downwards:

    for exponent in range (4, -1, -1):
        if size > 1024 ** exponent:
            unit_name = unit_names[exponent]
            return str(round(size / 1024 ** exponent)) + unit_name
    raise ImpossibleError

Same (almost same) applies to size_in_bytes.

Hardcoding \ as a path delimiter seriously impairs the portability. Prefer os.path.sep.
Instead of returning a list, consider turning it into an iterator.
Re Unused variable 'subfolders', a pythonic way to tell that the variable is truly unused is to call it _. I don't know if

for folder_name, _, filenames in os.walk(root_folder):

would pacify PyLint, but it would definitely make reviewer happier.


Efficiency of moving ball Here is another example of a moving ball.  I would like to see how to make it more efficient and better. I think that moving the variables to the top of the function would be better but then I tried this and it would not work as good as it did.

<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>CH5EX9: Moving In Circle</title>
<script src="modernizr.js"></script>
<script type="text/javascript">
window.addEventListener('load', eventWindowLoaded, false);
function eventWindowLoaded() {
    canvasApp();

}

function canvasApp() {
    var radius = 100;
    var circle = {centerX:250, centerY:250, radius:125, angle:0}
    var ball = {x:0, y:0,speed:.1};

    theCanvas = document.getElementById('canvasOne');
    context = theCanvas.getContext('2d');

  function  drawScreen () {

        context.fillStyle = '#EEEEEE';
        context.fillRect(0, 0, theCanvas.width, theCanvas.height);
        //Box
        context.strokeStyle = '#000000';
        context.strokeRect(1,  1, theCanvas.width-2, theCanvas.height-2);

        ball.x = circle.centerX + Math.cos(circle.angle) * circle.radius;
        ball.y = circle.centerY + Math.sin(circle.angle) * circle.radius;

        circle.angle += ball.speed;

        context.fillStyle = "#000000";
        context.beginPath();
        context.arc(ball.x,ball.y,15,0,Math.PI*2,true);
        context.closePath();
        context.fill();
    }

    function gameLoop() {
            window.setTimeout(gameLoop, 20);
            drawScreen()
        }

    gameLoop();
}

</script>

</head>
<body>
<div style="position: absolute; top: 50px; left: 50px;">
   <canvas id="canvasOne" width="500" height="500">
    Your browser does not support the HTML 5 Canvas.
   </canvas>
</div>
</body>
</html>

Your Script

Handlers

Unless your handlers are called somewhere else, you can just embed them

window.addEventListener('load', function () {
    canvasApp();
});

requestAnimationFrame

There's this timer function called requestAnimationFrame that is better than setTimeout and setInterval. It's more optimized for animations, aiming 60fps when possible. But it's prefixed in other browsers, check for compatibility first.

(function gameLoop() {
    window.requestAnimationFrame(gameLoop);
    drawScreen();
    // All other loop functions here
}());

Common gotchas in JS

Never forget semi-colons, even when JS allows you to forget them.
Remember using var when declaring variables. Unless you have them in some upper scope, if you forget them, they automatically become globals - and that's bad practice.

Conclusion

There was little to go by, but here's what I got

function canvasApp() {
    var radius = 100;
    var circle = {
        centerX: 250,
        centerY: 250,
        radius: 30,
        angle: 0
    }
    var ball = {
        x: 0,
        y: 0,
        speed: .1
    };

    var theCanvas = document.getElementById('canvasOne');
    var context = theCanvas.getContext('2d');

    function drawScreen() {

        context.fillStyle = '#EEEEEE';
        context.fillRect(0, 0, theCanvas.width, theCanvas.height);

        //Box
        context.strokeStyle = '#000000';
        context.strokeRect(1, 1, theCanvas.width - 2, theCanvas.height - 2);

        ball.x = circle.centerX + Math.cos(circle.angle) * circle.radius;
        ball.y = circle.centerY + Math.sin(circle.angle) * circle.radius;

        circle.angle += ball.speed;

        context.fillStyle = "#000000";
        context.beginPath();
        context.arc(ball.x, ball.y, 15, 0, Math.PI * 2, true);
        context.closePath();
        context.fill();
    }

    (function gameLoop() {
        window.requestAnimationFrame(gameLoop);
        drawScreen();
        // All other loop functions here
    }());
}

window.addEventListener('load', function () {
    canvasApp();
});


Generic object pooler implementation for Unity prefabs 2 scripts:
    - ObjectPooler
    - PoolableObject

The ObjectPooler acts as a singleton manager of poolable objects. Any prefab can be instantiated using the pooler via:

    GameObject go = GetPooledObject(prefabReference)

You would use this rather than instantiate the object manually. If an object of that prefab type already exists and is inactive, it is returned instead of instantiating a new one. Currently I am unsure if I should SetActive(true) the gameobject when it is returned from the pool, as that would invoke Start() on that GameObject before its values can be updated.

ObjectPooler.cs:

using System;
using System.Collections.Generic;
using UnityEditor;
using UnityEngine;

/* This class is used for pooling objects and reusing objects for performance
 * This works best when lots of the same object are spawned multiple times
 * such as projectiles from a gun */
public class ObjectPooler : MonoBehaviour
{
    public static ObjectPooler instance;
    public List<PrePooledObjects> prePooledObjects;
    private Dictionary<string, Queue<GameObject>> dict = null;

    [Serializable]
    public struct PrePooledObjects
    {
        public GameObject gameObject;
        public int count;
    }

    void Awake()
    {
        if (instance == null)
        {
            instance = this;
            dict = new Dictionary<string, Queue<GameObject>>();
        }
        else
        {
            Destroy(this);
        }
    }

    private void Start()
    {
        List<GameObject> pooledObjects = new List<GameObject>();
        foreach (PrePooledObjects prePoolObj in prePooledObjects)
        {
            for (int i = 0; i < prePoolObj.count; i++)
            {
                pooledObjects.Add(GetPooledObject(prePoolObj.gameObject));
            }
        }

        foreach (GameObject go in pooledObjects)
        {
            go.SetActive(false);
        }
    }

    public GameObject GetPooledObject(GameObject go)
    {
        if (!dict.ContainsKey(go.name))
        {
            dict.Add(go.name, new Queue<GameObject>());
        }

        if (dict[go.name].Count > 0)
        {
            return dict[go.name].Dequeue();
        }
        else
        {
            GameObject newGo = Instantiate(go);
            PoolableObject po = newGo.GetComponent<PoolableObject>();
            if (po == null)
            {
                po = newGo.AddComponent<PoolableObject>();
            }
            po.prefab = go;
            return newGo;
        }

    }

    public void ReleasePooledObject(PoolableObject po)
    {
        if (!dict.ContainsKey(po.prefab.name))
        {
            dict.Add(po.prefab.name, new Queue<GameObject>());
        }
        dict[po.prefab.name].Enqueue(po.gameObject);
    }
}

PoolableObject.cs:

using System;
using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public class PoolableObject : MonoBehaviour
{
    [SerializeField]
    public GameObject prefab = null;

    private void OnDisable()
    {
        if (prefab != null)
            ObjectPooler.instance.ReleasePooledObject(this);
    }
}
```

Interesting code! Am about to write something similar for a quite extreme case (decoration elements for procedurally generated terrain) so I could share a few thoughts I had:

That mechanism for prePooledObjects feels a bit too entwined with the pooling system itself. Why not provide a single function like:

PrePoolObjects(GameObject go, int count);

That would directly prepool a number of objects without the need for that extra struct and all that. Your concept is only necessary if you really want to provide the information what needs to be prePooled a single time and also clear the pool regularly for some reason.

Consider adding more than one new object when the queue is empty. For memory access reasons, it tends to be faster to instantiate multiple instances one after another than to instantiate multiple times at separate times.

Together with the above, it might make sense to shrink the number of pooled objects over time. At least if there are rare moments when a ton of a particular object is needed.

A really, really smart pooling system would evaluate the size of the pool constantly in the background (for example once every 5 physics updates) and ensure that there are not too few and not too many objects held "in stock". Important for that is a hysteresis, because of course you do not want to immediately grow the pool whenever a single instance was removed from it and vice versa. Only when it is too full or too empty for a certain period of time, it would start destroying or creating instances. Such a system would nicely adapt to the current need and not load up the memory when a single level progresses.
Admittedly this concept might be more necessary for my infinite-terrain game than for a game that's based on levels where you can flush the pool when switching scenes.

Last but not least: Using .name and perform that dictionary search every time could be replaced with custom constants that just hold an int and are the indices in an array/list of pool queues.
Then instead of providing the prefab to the GetPooledObject call, you provide that int constant and it takes the relevant queue from the array/list without any lookup operation.
The downside of this concept is the slight inconvenience of not being able to use prefabs immediately but first needing to define that constant. Also that might make using drag and drop with the prefabs more complicated or impossible.


Swing UI for a Virtual LaunchPad I'm currently coding for a school project. I'm planning on creating a virtual and simple Launchpad. Here's the first part of my code which treats the graphical interface. I know it's not well organized and that's why I ask for your help and tips!

import java.awt.EventQueue;
import java.awt.GridLayout;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JMenu;
import javax.swing.JMenuBar;

import javax.swing.JPanel;

public class clavier extends JFrame {

private JFrame frame = new JFrame("Clavier");
private JPanel parent = new JPanel(new GridLayout(0, 1));
private JPanel[] panel;
private JButton[][] button;
private static final String[][] key = {
      {"A", "Z", "E", "R", "T", "Y", "U", "I",
        "O", "P"}, {
        "Q", "S", "D", "F", "G", "H", "J", "K", "L", "M"}, {"W", "X", "C", "V", "B",
        "N"},
    };
private JMenuBar menuBar = new JMenuBar();
private JMenu m1 = new JMenu("Affecter les sons");
private JMenu m2 = new JMenu("Relancer");
private JMenu m3 = new JMenu("Aide");
private JButton b1 = new JButton("Play");
private JButton b2 = new JButton("Stop");

public clavier() {
    super("Clavier");
    panel = new JPanel[6];
    for (int ligne = 0; ligne < key.length; ligne++) {
        panel[ligne] = new JPanel();
        button = new JButton[20][20];
        for (int column = 0; column < key[ligne].length; column++) {
            button[ligne][column] = new JButton(key[ligne][column]);
            panel[ligne].add(button[ligne][column]);
        }
        parent.add(panel[ligne]);
    }

    this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    this.setLocationRelativeTo(null);
    this.menuBar.add(m1);
    this.menuBar.add(m2);
    this.menuBar.add(m3);
    this.setJMenuBar(menuBar);
    parent.add(b1);
    parent.add(b2);
    this.setVisible(true);
    add(parent);
    pack();
  }

public static void main(String[] args) {

    EventQueue.invokeLater(new Runnable() {
        @Override
        public void run() {
            clavier gui = new clavier();
        }
    });

}
}

Naming conventions

Classnames start with a capital letter. So it should be public class Clavier.

Consistent English or Frensh. Mixing makes things weird:
ligne => line
Clavier => Keyboard

Give variables meaningful names:
m1 => soundMenu
m2 => restartMenu
m3 => helpMenu
b1 => playButton
b2 => stopButton
button => keyboardButtons / letterButtons
key => keyboardLetters

I'd also rename ligne to row to fit better with the column.

Responsibility

What exactly is a Clavier in your program?
Is it a window with some content?
Is it a main screen that you put inside a window?
Is it just the keyboard, or is it a panel that contains a keyboard?

Right now, it's all of those things.

Let's remove all the Frame parts from this class. Some other class will be responsible of putting the keyboard into a frame to show it.
Let's also turn the keyboard into a JPanel instead of letting it reference a panel (parent). That way we can just put the Keyboard directly into a frame.

The biggest problem we'll get with this change is that we don't have a frame anymore to ade the menuBar to. So let's also create the new class that creates the frame, adds the main menu bar and a keyboard.

Our 2 new classes look like this:

public class Application {
    private JMenuBar menuBar = new JMenuBar();
    private JMenu soundMenu = new JMenu("Affecter les sons");
    private JMenu restartMenu = new JMenu("Relancer");
    private JMenu helpMenu = new JMenu("Aide");

    public Application() {
        this.menuBar.add(soundMenu);
        this.menuBar.add(restartMenu);
        this.menuBar.add(helpMenu);
    }

    public void run(){
        JFrame frame = new JFrame("Clavier");
        frame.setJMenuBar(menuBar);
        frame.add(new Keyboard());
        frame.pack();
        frame.setVisible(true);
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    }

    public static void main(String[] args) throws Exception {
        new Application().run();
    }
}

public class Keyboard extends JPanel {

    private JPanel[] keyboardPanel;
    private JButton[][] keyboardButtons;
    private static final String[][] keyboardLetters = {
            {"A", "Z", "E", "R", "T", "Y", "U", "I",
                    "O", "P"}, {
            "Q", "S", "D", "F", "G", "H", "J", "K", "L", "M"}, {"W", "X", "C", "V", "B",
            "N"},
    };

    private JButton playButton = new JButton("Play");
    private JButton stopButton = new JButton("Stop");

    public Keyboard() {
        super(new GridLayout(0, 1));
        keyboardPanel = new JPanel[6];
        for (int row = 0; row < keyboardLetters.length; row++) {
            keyboardPanel[row] = new JPanel();
            keyboardButtons = new JButton[20][20];
            for (int column = 0; column < keyboardLetters[row].length; column++) {
                keyboardButtons[row][column] = new JButton(keyboardLetters[row][column]);
                keyboardPanel[row].add(keyboardButtons[row][column]);
            }
            this.add(keyboardPanel[row]);
        }
        this.add(playButton);
        this.add(stopButton);
        this.setVisible(true);
    }
}

The next steps from here are adding the required listeners to all the buttons and menu items. This is actually the hard part. When doing those you also want to ask yourself: which class is responsible for what?
I'd say the keyboard is responsible for deciding what button is pressed. And then to pass it on to some class that knows what to do depending on which button was pressed.


Case insensitive string to replace custom comparers In some solutions I use case-insensitive strings a lot and so far I've been always writing custom collections or using custom comparers to support them. This is usually a lot of work and a lot of testing so I thought what if the string was case-insensitive instead?

The implementation is really simple because the CaseInsensitiveString is just a wrapper for a string that is using the StringComparer.OrdinalIgnoreCase comparer internally. Can you think of any improvements? I use it for hash-sets or dictionaries or anything else that might require case insensitive strings.

public class CaseInsensitiveString : IEquatable<CaseInsensitiveString>, IEquatable<string>
{
    private readonly string value;

    public CaseInsensitiveString() { }

    public CaseInsensitiveString(string value)
    {
        this.value = value;
    }

    private CaseInsensitiveString(CaseInsensitiveString other)
    {
        value = other?.value;
    }

    public static CaseInsensitiveString Empty => new CaseInsensitiveString(string.Empty);

    public override int GetHashCode()
    {
        return StringComparer.OrdinalIgnoreCase.GetHashCode(value);
    }

    public override bool Equals(object obj)
    {
        return
            (obj is CaseInsensitiveString cis && Equals(cis)) ||
            (obj is string s && Equals(s));
    }

    public bool Equals(CaseInsensitiveString other)
    {
        if (ReferenceEquals(other, null)) return false;
        return ReferenceEquals(this, other) || Equals(other.value);
    }

    public bool Equals(string other)
    {
        return StringComparer.OrdinalIgnoreCase.Equals(value, other);
    }

    public static implicit operator string(CaseInsensitiveString obj) => obj.value;

    public static implicit operator CaseInsensitiveString(string value) => new CaseInsensitiveString(value);

    public static bool operator ==(CaseInsensitiveString left, CaseInsensitiveString right) => new CaseInsensitiveString(left).Equals(right);

    public static bool operator !=(CaseInsensitiveString left, CaseInsensitiveString right) => !(left == right);
}

Example usage:

var set = new HashSet<CaseInsensitiveString>();
set.Add("foo").Dump(); // true
set.Add("fOo").Dump(); // false

var s = (CaseInsensitiveString)"foo";
(s == "foo").Dump(); // true
(s == "fOo").Dump(); // true

Creating a throw-away object just to perform == comparison is ugly,
and likely inefficient:

public static bool operator ==(CaseInsensitiveString left, CaseInsensitiveString right) => new CaseInsensitiveString(left).Equals(right);

I suspect you wanted to avoid a tedious null-check, but I don't think you have a choice:

public static bool operator ==(CaseInsensitiveString left, CaseInsensitiveString right) => !ReferenceEquals(left, null) && left.Equals(right);

But there's a more serious problem.
As it stands, two null instances of CaseInsensitiveString are not equal, and it should be.


Simple (Very Easy to Make) RPG Game Simulation in Python and Turtle The code creates a very simple/easy RPG game, with 2 classes Jedi and Orc. The data is visualized using turtle. Each class has a method of attack (lightsaber_attack for Jedi), which has an argument that must be either a Jedi or Orc instance. The .health attribute of the attacked one will be reduced by .power of the attacker. If .health is not positive, then the image of the character will disappear. By design, each character can attack itself.

Simulation

luke.lightsaber_attack( orc_1 )
luke.lightsaber_attack( orc_2 )
orc_1.attack( luke )
orc_2.attack( orc_2 )

Questions

How can I make the code to be easily understood by teenagers? (in a tutorial)
How can I make it more compact?
Are there any missing important features of Python's OOP that are important to be explained to students? (other than super and inheritance)

Image Links

jedi.gif
orc.gif
darkorc.gif
damaged.gif

Full code

import turtle
import time

jedi_gif = "/home/asus/Arief_tempo/images/random/jedi.gif"
orc_gif = "orc.gif"
darkorc_gif = "darkorc.gif"
damaged_gif = "damaged.gif"

turtle.register_shape( jedi_gif )
turtle.register_shape( orc_gif )
turtle.register_shape( darkorc_gif )
turtle.register_shape( damaged_gif )

class JediLuke:
    def __init__(self):
        self.power = 300
        self.health = 300

        self.img = turtle.Turtle( shape = jedi_gif )
        self.damaged_img = turtle.Turtle( shape = damaged_gif, visible = False )

        self.img.penup()
        self.damaged_img.penup()

    def lightsaber_attack(self, enemy):
        self.img.setpos(enemy.img.pos()[0], enemy.img.pos()[1])
        enemy.damaged_img.showturtle()
        enemy.health += - self.power
        time.sleep(1)

        enemy.damaged_img.hideturtle()
        if enemy.health < 0:
            enemy.img.hideturtle()

        self.img.setpos(200, 0)

    def change_pos(self, pos):
        self.img.setpos(pos[0], pos[1])
        self.damaged_img.setpos(pos[0], pos[1] + 150)

class Orc:
    def __init__(self, health, gif_image):
        self.power = 100
        self.health = health

        self.img = turtle.Turtle( shape = gif_image )
        self.damaged_img = turtle.Turtle( shape = damaged_gif, visible = False )

        self.img.penup()
        self.damaged_img.penup()

    def attack(self, enemy):
        current_pos = self.img.pos()
        self.img.setpos(enemy.img.pos()[0], enemy.img.pos()[1])
        enemy.damaged_img.showturtle()
        enemy.health += - self.power

        time.sleep(1)

        enemy.damaged_img.hideturtle()
        if enemy.health < 0:
            enemy.img.hideturtle()

        self.img.setpos(current_pos[0], current_pos[1])

    def change_pos(self, pos):
        self.img.setpos(pos[0], pos[1])
        self.damaged_img.setpos(pos[0], pos[1] + 150)

luke = JediLuke()
luke.change_pos( [200, 0] )

orc_1 = Orc( health = 400 , gif_image = orc_gif)
orc_1.change_pos( [-200, 100] )

orc_2 = Orc( health = 200, gif_image = darkorc_gif )
orc_2.change_pos( [-200, -100] )

jedi_gif = "/home/asus/Arief_tempo/images/random/jedi.gif"

It's unclear why this image has an absolute path but no others do. They should probably all be relative, as the other three are.

Especially if this is for a tutorial, you need to add docstrings to all of your functions.

This:

self.img.setpos(enemy.img.pos()[0], enemy.img.pos()[1])

can use argument expansion, i.e.:

self.img.setpos(*enemy.img.pos())

That pattern can be used elsewhere you're indexing into the position.

This:

enemy.health += - self.power

should be

enemy.health -= self.power


List iteration, creation, or comprehension I suspect that this can be done in a much neater way using list comprehension in python:

poses = []
for x,y in get_points(spline):
    pose = Pose2D()
    pose.theta = 0
    pose.x=x
    pose.y=y
    poses.append(pose)
return poses

Is there a way to create a nice one liner for the above? (I do not have a constructor that takes the three arguments for Pose2D)

def make_pose(pt):
    pose = Pose2D()
    pose.theta = 0
    pose.x, pose.y = pt
    return pose

poses = [make_pose(pt) for pt in get_points(spline)]

Alternatively, you could subclass Pose2D to give it an appropriate constructor.


Program that finds prime numbers in a specific range I'm kinda new to programming, and as a solution to an exercise, I made a program to find prime numbers in a range. It runs just right for small ranges of numbers, but for this exercise we are given a high range of nums, and it just takes much time to finish examining. Any suggestions how can I make it faster?

#include <stdio.h>

#define START 190000000
#define END 200000000

int main()
{
    int primenum = 0, i = 0, j = 0, c = 0;
    for (i = START; i <= END; i++)
    {
        printf("EXMINING %d\r\n", i);
        c = 2;
        for (j = 2; j <= i-1; j++)
        {
            if (i%j == 0)
            {   c=1;
                break;
            }
        }
        if (c == 2) primenum = primenum + 1;
        printf("Prime Numbers Found so far: %d\r\n", primenum);
    }
    printf("THE PRIME NUMBERS ARE %d", primenum);
    return 0;
}

I combine here all the above and look up tables.

Use the threshold of sqrt(test_prime) to shrink the range to be tested, as said by @Gaurav.
Increase the prime number to be tested by 2, as said by @1201ProgramAlarm.
Use a look up tables to check only with the prime numbers we have detected until that moment (we remove a lot of unnecessary checks).
Load/Save the look up table for future executions.
Use SIMD instrinsics (not implemented in this solution), so that you can check 4 primes into the look up table at the same time.

My tests took about 4 minutes without pre-calculated lookup table, and about 30 seconds using pre-calculated lookup table.

The code here

#include <stdio.h>
#include <stdlib.h>
#include <math.h>
#include <string.h>

#define START 190000000
#define END 200000000

int is_prime(long test, long n_primes, long* list_primes) {
    long max = sqrt(test);
    long index = 1;
    long prime = list_primes[index];
    while (prime <= max) {
        if (test % prime == 0)
            return 0;
        if (++index >= n_primes)
           break;
        prime = list_primes[index];
    }
    return 1;
}

void append_prime(long prime, long* size, long* n_primes, long** list_primes) {
    if (*n_primes == *size) {
        *list_primes = (long*)realloc(*list_primes, (*size + 4096)*sizeof(long));
        *size += 1024;
    }
    (*list_primes)[*n_primes] = prime;
    *n_primes += 1;
}

int load_from_disk(long* size, long* n_primes, long** list_primes) {
    FILE* f = fopen("primes.dat", "rb");
    if (f == NULL)
        return 0;
    fread(size, sizeof(long), 1, f);
    fread(n_primes, sizeof(long), 1, f);
    *list_primes = (long*)malloc( ( (*n_primes + 4095) / 4096 ) * 4096 * sizeof(long) );
    fread(*list_primes, sizeof(long), *n_primes, f);
    fclose(f);
    f = NULL;
    return 1;
}

int save_to_disk(long* size, long* n_primes, long** list_primes) {
    FILE* f = fopen("primes.dat", "w");
    if (f == NULL)
        return 0;
    fwrite(size, sizeof(long), 1, f);
    fwrite(n_primes, sizeof(long), 1, f);
    fwrite(*list_primes, sizeof(long), *n_primes, f);
    fclose(f);
    f = NULL;
    return 1;
}

long find_primes_until(long threshold, long* size, long* n_primes, long** list_primes) {
    if (!load_from_disk(size, n_primes, list_primes)) {
        *size = 4096;
        *n_primes = 0;
        *list_primes = (long*)malloc((*size) * sizeof(long));
        memset(*list_primes, 0, (*size) * sizeof(long));

        if (threshold > 2) {
            (*list_primes)[(*n_primes)++] = 2;
        } else {
            return *n_primes;
        }

        if (threshold > 3) {
            (*list_primes)[(*n_primes)++] = 3;
        } else {
            return *n_primes;
        }

        if (threshold > 5) {
            (*list_primes)[(*n_primes)++] = 5;
        } else {
            return *n_primes;
        }

        if (threshold > 7) {
            (*list_primes)[(*n_primes)++] = 7;
        } else {
            return *n_primes;
        }
    }

    long prime = (*list_primes)[(*n_primes)-1] + 2;
    while (prime < threshold) {
        //printf("Examining number: %ld / %ld      \r", prime, threshold);
        if (is_prime(prime, *n_primes, *list_primes)) {
            //printf("\nPrime number found: %ld\n", prime);
            append_prime(prime, size, n_primes, list_primes);
        }
        prime += 2;
    }
    save_to_disk(size, n_primes, list_primes);

    return *n_primes;
}

void find_primes_interval(long start, long end)
{
    long* list_primes = NULL;
    long  size = 0;
    long  n_primes = 0;

    find_primes_until(start, &size, &n_primes, &list_primes);

    if ((start & 0x01) == 0)
        start++;
    while (start < end) {
        printf("Examining number: %ld       \r", start);
        if (is_prime(start, n_primes, list_primes)) {
            printf("\nPrime number found: %ld\n", start);
            append_prime(start, &size, &n_primes, &list_primes);
        }
        start += 2;
    }
}

int main()
{
    find_primes_interval(START, END);
    return 0;
}


Insert, update and get in the same php file? I'm creating a CRUD page. My first approach was use the same class editCategory.php for doing these actions:

If this file is called via GET and categoryId parameter doesn't exist -> shall show a empty form
If this file is called via GET and categoryId parameter is provided -> shall show a form with the data of this category.
If this file is called via POST and there is no categoryId parameter -> shall create a new category.
If this file is called via POST and categoryId parameter is provided -> shall update this category.

The code works ok, but I have the sense that the code is cluttered and that it could be organized better.

    <?php
        require_once "../include_dao.php";
        $action = isset($_POST["action"]);

        $category = new Category();
        $categoryDao = new CategoryMySqlDAO();

        $categoryName = "";
        if ($action == "save") {                                        // DO_POST
            $categoryName = $_POST["name"];
            $category->name=$categoryName;

            if (isset($_POST["categoryId"])) {                              // update
                $categoryId = (int) $_POST["categoryId"];
                $category->categoryId=$categoryId;
                $categoryDao->update($category);
            } else {                                                        // insert
                $categoryDao->insert($category);
            }
            $messageSuccess = "Category saved";
        } else {                                                        // DO_GET
            if (isset($_GET["categoryId"])) {
                $categoryId = (int) $_GET["categoryId"];

                $category = $categoryDao->load($categoryId);
                $categoryName = $category->name;
            }

        ?>
<html>
 HTML code here showing the form
</html>

Your concept using the HTTP verbs is pretty much the idea behind REST. You could go one step further and use the less known verbs like PUT, DELETE, UPDATE.

Also u could use the $_SERVER['REQUEST_METHOD'] to switch between the HTTP modes you would like to process.

I would suggest to write a "class Request" to be reused by all your CRUD pages or use / get inspired by (for example) symfony2

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/Request.php

u are using classes like CategoryMySqlDAO why do not u use a CRUD class? u could implement an abstract class CRUD with "create, update, delete" methodes and then use subclasses for each crud page. Then you would inject the DAO and stuff in the constructor and use autoload to get them instead of a smelly required statement in each file.

also having HTML and logic in the same file never is a good idea.

i hope i could give you some inspiration how to improve your current state


Sandbox or safely execute eval In a large project of mine, I've run into a situation where a client might need to run evaluated JavaScript code. I know, it makes me cringe too. One option is to manually parse it, but for future flexibility, I would like to evaluate the code safely.

Everything I've seen and researched says, NO. Ah, the nay-sayers.

This is my attempt to sandbox the eval:

jsFiddle

Can you shoot holes in it and try and break it? I should wrap it in a try so it doesn't throw ugly errors, but beyond that, tell me where I have gone awry.

/***************************************
 * Senica Gonzalez (senica@gmail.com)
 * This is an attempt to sandbox an eval
 * in javascript using an iframe.
 * Test it and let me know if you
 * can break it.
 ***************************************/

window.addEventListener("message", function(event){
    $('#result').text(event.data.eval);
    console.log(event.data.scope1);
    console.log(event.data.scope2);
}, false);

test = 'do I exist in the window?';

var input = $('[name=toeval]');
input.on('change', function(){
    var val = $(this).val();
    var code = btoa('<html>\
        <head><\/head>\
        <body>\
            <' + 'script>\
                var party_size = 10;\
                window.parent.postMessage({\
                    eval: eval('+val+'),\
                }, "*");\
            <\/script' + '>\
        <\/body>\
    <\/html>');
    var frame = $('<iframe sandbox="allow-scripts" src="data:text/html;base64,'+code+'"></iframe>');
    var sandbox = $('#sandbox');
    sandbox.html(frame);
});

I have been unsuccessful at accessing the parent document variables, or being able to do anything obnoxious other than to my self within the sandbox.

Yes, I can poke a few holes in this.

Firstly, your eval() isn't actually doing much of anything here. From the perspective of the parser running the script, that line looks like this:

eval(party_size < 5)

This means that party_size < 5 is evaluated to false before it is passed to the eval() function. eval() then just spits this value right back out.

Some consequences of this:

It will fail to act as expected if the expression evaluates to a string value. For example, if you put "a" + "b" into your textbox, this will fail with the error ReferenceError: ab is not defined, because the JS engine will be trying to evaluate eval("ab").
It will fail if the code is anything other than a single expression. So entering this into the textbox: var a = 2; a; fails with a syntax error, even though this is a perfectly legitimate thing to pass to eval().

So let's assume that was just a silly oversight and you meant to do this:

eval: eval("'+val+'"),\

Well, this will succeed in actually executing chunks of code, until that code contains a quotation mark:

eval("var a = "a"; a + "b"");

at which point it will be broken again.

Now let's imagine you decided to get around this by changing this line to:

eval: eval("' + val.replace(/"/g, '\\"') + '"),\

Now you can be pretty confident that the code will run (in most cases) and that all you need to worry about at this point is malicious code (haha).

In terms of malicious code, the attacker could execute something resource intensive that hogs the CPU and RAM:

var a = []; while(true) { a.push(new Date()); console.log("gotcha!"); }

Or they might choose to carry out a CSRF-like attack:

var x = new XMLHttpRequest();
x.open("post", "https://www.mybank.com/transferMoney");
x.send("amount=10000&toAccount=34567890");

So keeping the host DOM safe is really not the only thing to worry about. There are any other number of malicious things an attacker can carry out if they're able to run code on someone's machine.

Given the above, I would stick with the assertion that eval is evil, and it's not going to be easy to find a "safe" way to execute untrusted code.


Check string value of boolean to a mapped enum In a project, a boolean can be mapped to an enum to display a custom string for true and false.
If I have a string interpretation of a value, I have a method that is checking against the custom enum names or/and against regular boolean expressions like true, false, 0 and 1.

The method looks like this

public Boolean createParameterObjectValue(String value) {
    if (getMetaData().getValidValues().isAvailable() && getMetaData().getValidValues().getType() == ValidValues.Type.ENUM) {
        if (getMetaData().getValidValues(EnumValidValues.class).getEnumName(1).equalsIgnoreCase(value)) {
            return true;
        }
        if (getMetaData().getValidValues(EnumValidValues.class).getEnumName(0).equalsIgnoreCase(value)) {
            return false;
        }
        //TODO refactor redundance
        return !("0".equals(value) || "FALSE".equalsIgnoreCase(value));
    } else {
            return !("0".equals(value) || "FALSE".equalsIgnoreCase(value));
    }
}

First I'll check if the object in my project (a parameter) that is handling the boolean value, has defined values and has an enum mapping.

If this is true, compare the value to the enum names. If none of them equals the given value,I assume that the given string is "0","1","false" or "true". The same happens if the parameter does not have an enum mapping.

As you see from the TODO comment I'm not really happy with the redundancy, even if it doesn't really affect the performance. How could I remove the redundancy from this code?

The redundant logical expression seems to be too complex for this case.

And there is probably a bug. You say that the expected set of values for it is ("0", "1", "false", "true"), but if value = "2", it will also evaluate to "true":

!("0".equals("2") || "FALSE".equalsIgnoreCase("2")); ->

!(false || false) -> !false -> true

"2" is weird for "true" value.

So I suggest to reformulate the expression to

Boolean.parseBoolean(value) || "1".equals(value)

"1" is still there, because parseBoolean checks for equivalence for "true" string value.

The logic of the entire method can be thus reformulated with improved readability and simplified:

public boolean createParameterObjectValue2(String value) {
  return Boolean.parseBoolean(value) ||
         "1".equals(value) ||
         evaluatesToTrueFromMetadata(value);
}

private boolean evaluatesToTrueFromMetadata(String value) {
  return getMetaData().getValidValues().isAvailable() &&
      getMetaData().getValidValues().getType() == ValidValues.Type.ENUM &&
      getMetaData().getValidValues(EnumValidValues.class).getEnumName(1).equalsIgnoreCase(value);
}

getMetaData().getValidValues() in the private method should also be extracted to a local reference in order to shorten the expression.


Parse bed file with Pandas For the sections between count=1s and the start and end; combine overlapping positions and output the median of the counts.
Input
chr         start   stop   strand   count
chr1        0       13320   -       1
chr1        13320   13321   -       2
chr1        13321   13328   -       1
chr1        13328   13342   -       2
chr1        13342   13343   -       18
chr1        13343   13344   -       36
chr1        13344   13345   -       18
chr1        13345   13346   -       6
chr1        13346   16923   -       1
chr1        16923   16942   -       3
chr1        16942   16943   -       2

Output
chr1        13320   13321    2
chr1        13328   13346   18
chr1        16923   16943   2.5

For the second value:

Start 13328 - this is because the 4th value in the table has the start 13328.
This is the row after the second count=1.
Stop 13346 - this is because the 8th value in the table has the stop 13346.
This is the row before the third count=1.
Count 18 - this is the median of counts between the 4th and 8th inclusive.

Here is my code.
from pathlib import Path
import pandas as pd
file = Path("bed_file.bed")
# load with pandas
df = pd.read_csv(file, sep='\t', header=None)

# set colnames
header = ['chr','start','stop','strand','count']
df.columns = header[:len(df.columns)]

# index where count=1
col_count = df['count'].tolist()
li = [i for i, n in enumerate(col_count) if n == 1]

# create new dataframe
newDF = pd.DataFrame(columns=['chr','start', 'stop', 'count'])
# last position
end = df.index[-1]

# parse dataframe
for idx, elem in enumerate(li):
    if elem != li[-1]:
        next_elem = li[(idx + 1) % len(li)] # next element where count=1
        start = df.iloc[elem]['stop'] # start position
        stop = df.iloc[next_elem-1]['stop'] # stop position
        if next_elem - (elem+1) == 1: # cases where only one position and we cannot compute median
            count = df.iloc[elem+1]['count']
            #print(f"start={start}\tstop={stop}\tcount={count}")
        else:
            count = df.iloc[elem+1:next_elem]['count'].median()
            #print(f"start={start}\tstop={stop}\tcount={count}")
        newDF = newDF.append({
            'chr' : df.loc[0,'chr'],
            'start' : start,
            'stop' : stop,
            'count' : count

        },ignore_index=True)
    else: # last element in the list
        start = df.iloc[elem]['stop']
        stop = df.iloc[end]['stop']
        count = df.iloc[elem+1:end+1]['count'].median()
        #print(f"start={start}\tstop={stop}\tcount={count}")
        newDF = newDF.append({
            'chr' : df.loc[0,'chr'],
            'start' : start,
            'stop' : stop,
            'count' : count
        },ignore_index=True)

Is there a better way to do this? Is my code Pythonic?

I'll first offer some critique of your code, and then I'll show you how I would approach the problem.

Commented out code should be removed before asking for a code review #print(f"start={start}\tstop={stop}\tcount={count}")
Many of the comments don't add value. # last position doesn't mean much on its own. Why do you want the last position? Why doesn't the code do a good enough job explaining that?
Generally an if/else in a loop where one of branches is only taken once, either at the start or end, can be removed. You can iterate less, and deal with the case explicitly. You can add a sentinel value so you don't have to check if you are at the end of the iterator. You can used the available libraries or built-in functions, which will deal with the case for you.

# load with pandas
df = pd.read_csv(file, sep='\t', header=None)

# set colnames
header = ['chr','start','stop','strand','count']
df.columns = header[:len(df.columns)]

# index where count=1
col_count = df['count'].tolist()
li = [i for i, n in enumerate(col_count) if n == 1]

If the header is cut short len(df.columns) < len(header), the first thing to be cut off is the column df['count']. You then assume it exists straight away after by using it. Which is it? Will it always exist, or sometimes will there not be enough columns? Erring on the side of it always exists, the code becomes
# load with pandas
df = pd.read_csv(file, sep='\t', names=('chr', 'start', 'stop', 'strand', 'count'), header=None)

# index where count=1
col_count = df['count'].tolist()
li = [i for i, n in enumerate(col_count) if n == 1]

# index where count=1
col_count = df['count'].tolist()
li = [i for i, n in enumerate(col_count) if n == 1]

...

for idx, elem in enumerate(li):

If you are using pandas (or numpy) it is generally not the best to move the data back and forth between the library and Python. You lose most of the efficiency of the library, and the code generally becomes far less readable.
Don't use names like li. It doesn't give any information to the reader. If you have a list of indices, what will you use the list for? That would make a much better name.
Using pandas more, and renaming gives something like
splitting_indices = df.index[df['count'] == 1].tolist()

for idx, elem in enumerate(splitting_indices):

if next_elem - (elem+1) == 1: # cases where only one position and we cannot compute median
    count = df.iloc[elem+1]['count']
    #print(f"start={start}\tstop={stop}\tcount={count}")
else:
    count = df.iloc[elem+1:next_elem]['count'].median()

Finding this logic in amongst getting the data out from the dataframe is not easy. This is the core logic, and should be treated as such. Put this in a function at the very least.
def extract_median(df, elem, next_elem):
    if next_elem - (elem+1) == 1: # cases where only one position and we cannot compute median
        count = df.iloc[elem+1]['count']
    else:
        count = df.iloc[elem+1:next_elem]['count'].median()
    return count

Now it should be much more apparent that the comment is bogus. You CAN compute the median of a single element list. So why are we special casing this? df.iloc[elem+1:next_elem] works even if next_elem is only one bigger than elem+1.
def extract_median(df, elem, next_elem):
    return df.iloc[elem+1:next_elem]['count'].median()

And now we can see that a function is probably not necessary.

The approach I would take to implementing this is to try and stay using pandas as long as possible. No loops. No tolist. Since I won't want loops, indices are probably not needed too, so I can limit usage of iloc and df.index.
First, read in the data
df = pd.read_csv(file, sep='\t', names=('chr', 'start', 'stop', 'strand', 'count'), header=None)

     chr  start   stop strand  count
0   chr1      0  13320      -      1
1   chr1  13320  13321      -      2
2   chr1  13321  13328      -      1
3   chr1  13328  13342      -      2
4   chr1  13342  13343      -     18
5   chr1  13343  13344      -     36
6   chr1  13344  13345      -     18
7   chr1  13345  13346      -      6
8   chr1  13346  16923      -      1
9   chr1  16923  16942      -      3
10  chr1  16942  16943      -      2

Then, find every row of interest. That would be everywhere count is not 1.
df['count'] != 1

0     False
1      True
2     False
3      True
4      True
5      True
6      True
7      True
8     False
9      True
10     True

I want to group all the consecutive rows that are True together. The usual method to group consecutive rows by a column value is

Keep a running tally.
Compare each value in the column with the next one.
If they are the same, don't do anything.
If they are different, add 1 to a running tally.
Associate the tally to that value.
Groupby the tally.

In code
mask = df['count'] != 1
tally = (mask != mask.shift()).cumsum()

    count   mask  tally
0       1  False      1
1       2   True      2
2       1  False      3
3       2   True      4
4      18   True      4
5      36   True      4
6      18   True      4
7       6   True      4
8       1  False      5
9       3   True      6
10      2   True      6

Grouping then gives
df.groupby(tally).groups

{1: Int64Index([0], dtype='int64'),
 2: Int64Index([1], dtype='int64'),
 3: Int64Index([2], dtype='int64'),
 4: Int64Index([3, 4, 5, 6, 7], dtype='int64'),
 5: Int64Index([8], dtype='int64'),
 6: Int64Index([9, 10], dtype='int64')}

Since you only want the rows where count is not 1, we can reuse the mask to filter them out.
df[mask].groupby(tally).groups

{2: Int64Index([1], dtype='int64'),
 4: Int64Index([3, 4, 5, 6, 7], dtype='int64'),
 6: Int64Index([9, 10], dtype='int64')}

And finally the median is quick to get from a grouper
df[mask].groupby(tally).median()

         start     stop  count
count
2      13320.0  13321.0    2.0
4      13343.0  13344.0   18.0
6      16932.5  16942.5    2.5

In the end, the code is a lot shorter
df = pd.read_csv(file, sep='\t', names=('chr', 'start', 'stop', 'strand', 'count'), header=None)
mask = df['count'] != 1
tally = (mask != mask.shift()).cumsum()
df[mask].groupby(tally).median()


Prints out all ways to multiply smaller integers that equal the original number Write a program in Java that takes a positive integer and prints out all ways to multiply smaller integers that equal the original number, without repeating sets of factors.  In other words, if your output contains 4 * 3, you should not print out 3 * 4 again as that would be a repeating set.  Note that this is not asking for prime factorization only.

Here is the sample from one solution:

Example:

$ java -cp . PrintFactors 32

  32 * 1

  16 * 2

  8 * 4

  8 * 2 * 2

  4 * 4 * 2

  4 * 2 * 2 * 2

  2 * 2 * 2 * 2 * 2

I'm looking for code review, best practices, optimizations etc.

Verifying Complexity: \$O(n!)\$ - time, where \$n\$ is the input number.

public final class PrintFactors {

    private PrintFactors() {}

    public static void printFactors(int number) {
        if (number <= 0) throw new IllegalArgumentException("The number should be greater than 0.");
        printFactorsList(number, number + "*" + 1 + "\n", number);
    }

    private static void printFactorsList(int dividend, String factorString, int prevDivisor) {
        /*
         * This function contains factorString as an argument to facilitate the recursive call for subsequent
         * factors until it reaches prime values. For example, let's say input number = 32 and when i = 8 it prints
         * 8*(32/8) ==> 8 * 4 but the subsequent reduction of 4 is needed and this is done by recursively passing in 4
         * as number. But we also want to maintain the chain "8 * ". So this makes the carry over string as an input
         * argument for the helper function printFactorsList
         */
        for (int divisor = dividend - 1; divisor >= 2; divisor--) {

            if (dividend % divisor != 0)
                continue;

            if (divisor > prevDivisor)
                continue;

            int quotient = dividend / divisor;

            /*
             * 32*1 16*2 8*4 8*2*2 4*4*2 4*2*2*2 2*2*2*2*2
             *
             * Note: as we go right, the values keeps descending.
             */
            if (quotient <= divisor) {
                if (quotient <= prevDivisor) {
                    System.out.println(factorString + divisor + "*" + quotient);
                }
            }
            printFactorsList(quotient, factorString + divisor + "*", divisor);
        }
    }

    public static void main(String[] args) {
        printFactors(12);
        System.out.println();
        printFactors(32);
    }
}

public static void printFactors(int number) {
  if (number <= 0) {

You should include the number into the exception's text.

    throw new IllegalArgumentException("The number should be greater than 0.");
  }
  printFactorsList(number, number + "*" + 1 + "\n", number);
}

That's a bad name for a function.

private static void printFactorsList(int dividend, String factorString, int prevDivisor) {

This should be formatted as javadoc.

      /*
       * This function contains factorString as an argument to facilitate the recursive call for subsequent
       * factors until it reaches prime values. For example, let's say input number = 32 and when i = 8 it prints
       * 8*(32/8) ==> 8 * 4 but the subsequent reduction of 4 is needed and this is done by recursively passing in 4
       * as number. But we also want to maintain the chain "8 * ". So this makes the carry over string as an input
       * argument for the helper function printFactorsList
       */

Why not start this loop from prevDivisor?
Also, you can optimize this loop by storing numbers in factorized form and generating all divisors less than something on the fly. This will be much faster because you will iterate only over valid divisors. You can also memoize all solutions for a number and reuse them.

  for (int divisor = dividend - 1; divisor >= 2; divisor--) {

    if (dividend % divisor != 0) {
      continue;
    }

    if (divisor > prevDivisor) {
      continue;
    }

    int quotient = dividend / divisor;

          /*
           * 32*1 16*2 8*4 8*2*2 4*4*2 4*2*2*2 2*2*2*2*2
           *
           * Note: as we go right, the values keeps descending.
           */

Merge this two ifs into one.

    if (quotient <= divisor) {
      if (quotient <= prevDivisor) {
        System.out.println(factorString + divisor + "*" + quotient);
      }
    }
    printFactorsList(quotient, factorString + divisor + "*", divisor);
  }
}

public static void main(String[] args) {

You are not reading from anywhere. Is this the intended behavior?

  printFactors(32);
}