- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
Hey, never let complete ignorance of a subject prevent you from having strong opinions about it.
Admin
Admin
Popular Editable Ropy Language? Proper Enterprisy Real Lemon? Particularly Enduring Random Limit?
Admin
I love how every object is named objXXXXXX, even objImgData which is actually an array or primitives. The 'obj' prefix adds absolutely nothing, when most variables are objects, as they are in object oriented languages.
This practice of type-tagging variable names in a strongly typed language is a misunderstanding of the purpose of Hungarian notation. It was widely practiced in Microsoft for a while but they've repented.
Even if mutant-Hungarian notation was a good idea, the 'obj' prefix is the worst example of it. It is so generic that it is useless.
Admin
So you are assuming:
a) that all possible values of "filename" are mapped to URLs b) that "filename" and its corresponding URL can be determined at the point of page generation for all pages that contain an IMG reference to it
Or for all we know, this code IS part of the implementation of a static asset manager.
Now suppose SRC is set by the client, and the filename is determined by some parameters. Your proposed solution is:
As opposed to the current solution:
Admin
Admin
And the replacement code doesn't strip the metadata (localisation, serial number of camera, any html code that may be injected) introducing possible confidentiality/security breaches. The first code wasn't so bad if it wasn't so innefficient.
Admin
Seriously, I have no idea what you're trying to get at there. Restating your point using a synonym doesn't change anything.
Admin
Admin
b) You're making a big assumption that there might be any data in the file in the first place that shouldn't be sent to the client. Unless you have a legitimate reason to believe that's the case you can't criticise the code on that basis (sending the file as-is is what web servers do by default, after all, and you don't see people all up in arms about what a horrible security hole that is).
Admin
Way back in the day, graphics were stored as raw bitmaps in RAM (and sometimes on the disk too) because decoding compressed images was too complex a task for your "render bitmap" function to handle. Now, decoding compressed images is so goddamn trivial that your "render bitmap" function can support pretty much any format you'd like it to, saving you lots of RAM by allowing you to leave the images in their compressed form longer.
Admin
You still have to worry about them grabbing files that aren't images, though...
Admin
My point is that just because a few "standards" say something is okay, like uppercase elements and not closing tags, it doesn't mean that it's not an antipattern.
Admin
Also, to borrow an economics term, it's signaling. The majority of HTML code that's lowercase isn't too bad, while the majority of HTML code that's uppercase would probably end up on this site. It's not a cause, but it's definitely an indicator.
Admin
Admin
No, British economists do. American economists call it signaling.
Admin
HTML Tip: Use Lowercase Tags
HTML tags are not case sensitive:
means the same as
. Many web sites use uppercase HTML tags.
W3Schools use lowercase tags because the World Wide Web Consortium (W3C) recommends lowercase in HTML 4, and demands lowercase tags in XHTML.
http://www.w3schools.com/html/html_elements.asp
Admin
Admin
Admin
As another data point, the DOM spec considers upper-case tag names to be canonical for HTML documents, and requires that element objects return their tag name in that form. It states that attribute names are "typically" returned in lower-case, but it doesn't seem to actually require that as far as I can tell.
Admin
Well, in the comments here alone, you can see one guy that uses uppercase, and does not use the alt attribute. I, on the otherhand, use lowercase, and I did include the alt attribute.
Admin
It's minor compared to the accessibility issues caused by leaving out the alt attribute, and it is very nitpicky, but in my experience, uppercase HTML definitely is most used by the WTFers.
Do you shout your HTML?
Admin
Again, back to the competing spec XKCD. And it definitely is worse that it's a preference, not a requirement.
Can we just agree that it's preference, and that the real WTF in his comment was leaving out the alt attribute?
Admin
Admin
Admin
A class that keeps references to the data passed to its constructor is not odd or unexpected at all. Keeping the exact contents for reuse is not an "additional" action, it's practically a no-op.
If I was asked to write a "convert(input, outputFormat)" function, the first line I would immediately write would be "if (outputFormat == input.format) return input". I really don't understand why anyone expects the implementation to be inefficient and redundant when the efficient solution is so obvious and simple.
Admin
Having looked at the documentation, however, it seems that not only does it not decode the image data until required, it doesn't even read it (or at least not all of it) from the file - the documentation states that when constructed from a filename it keeps the file locked until the Bitmap is disposed, and when constructed from a stream it requires that the stream remain valid, again until the Bitmap is disposed. I definitely wouldn't have guessed this without seeing it documented, or unless I'd actually used the class and observed such behaviour.
YMMV, but if I didn't know otherwise I'd expect the constructor and Save method to work like a parser and serialiser for some sort of structured data, and IME those generally don't keep the original bytes/characters around. As an exaggerated example, you wouldn't expect Int32.Parse("1") and Int32.Parse("01") to return different things, or for stringification to magically reinstate the leading zero for the latter - doing the corresponding thing for images is more reasonable, but again, I'd still consider it non-obvious enough to be worth documenting. That's fair enough, but in this case it isn't a "convert" function - loading the original and saving it in the new format are separate steps, with no apparent reason for the save step to know anything about where the image came from to begin with (or at least the API makes it look that way - the actual implementation seems to be different, as per the above). Well, for one thing, it isn't necessarily inefficient or redundant if it doesn't do the same thing as the "efficient" alternative. We've already had one person on this thread who thought the "conversion" would remove potentially sensitive metadata, and more generally, it's not that unreasonable to expect such an operation to perform some kind of normalisation. And on the other hand, someone who does want the optimisation may be led to waste time reimplementing it themself, if they either don't realise that the Bitmap class already does it or don't want to rely on undocumented behaviour.Admin
After pondering it for a while, I wonder if it's more just a style of older developers that are more used to uppercase in their environment.
Admin
In the same sense, your Int32.Parse example is not applicable, because it is not a constructor or a method. Here Parse is a stand-alone function that returns an integer. There is a clear disconnect between the string parameter and the value returned. There is no state.
Yes it is redundant and inefficient, because the requirement of the method is to "Save this Image to the specified file in the specified format." When the specified format is the same as the Image's format, there is one implementation that is clearly more efficient: using the same data. Of course no one should rely on undocumented assumptions, I'm not saying that at all. On the contrary, other commenters were speaking authoritatively about something undocumented that they clearly had not researched, and I corrected them after running my own test and verifying the output. I do believe it is an unreasonable expectation, which is what prompted me to test it in the first place.And by the way, reimplementing the optimization himself wouldn't be a waste of time. That is still relying on undocumented implementation details, and it's not explicit to the code's reader or even necessarily forward-compatible. If you want to ensure that the exact file is copied, you should keep your own handle on the file contents and code that explicitly. And if you want to ensure that the metadata is removed, you should explicitly code that as well.
Admin
I'm starting to think you're missing the point on purpose....
Yes, and without explicit documentation, the "persistent state" could just as easily be the decoded image data, with no reference to the original file once the constructor finishes reading it. If you create a new ArrayList by passing an existing collection to the constructor, then the new instance doesn't depend on the existing collection, and it's entirely reasonable to expect Bitmap to work similarly. (I see there's also a static method to create an ArrayList that does behave that way, but it's not the default, and for good reason IMHO.) OK, so I create a function that takes a filename, constructs a Bitmap using the filename as the constructor argument, and returns it. I don't see how being a function^Wstatic method magically changes the expectation - in some languages there isn't even a real distinction between a constructor and a static method that returns a new object in the first place. I'm not sure what you mean here.... Yes, that's what I meant by reimplementing it. (I'm assuming that the code doesn't know when it reads the image which format it'll be saving it in, so it's not as simple as replacing the use of Bitmap with a plain read/write.) It's unnecessary to do that if Bitmap is guaranteed to do it itself, and if it isn't guaranteed, then that comes back to the original point, which is that without knowing explicitly, it's entirely reasonable to think that Bitmap might not behave that way.Admin
Hahahaha, I recognise this exact code from my previous job! I actually wrote the code that replaced it, which ended up being significantly simpler. This particular code was used in an ASP.NET Web Forms page and the new code I wrote to replace it went into an ASP.NET MVC action (basically just returned a FileResult).
There was no reason to load the image via the Bitmap constructor. Conversion was not necessary as the process that handled uploading the images guaranteed that they were JPEGs. The issue was that the files were located on an internal NAS and required access restrictions, so they had to be proxied for people to actually be able to see them. I would have liked to have a proper solution (like S3 with signed URLs) but didn't get allocated enough time to work on it :(
Admin
Again, you give a non-applicable example: You are talking about the reference to the collection that holds the data, not the actual data. The ArrayList does reference the same elements. Likewise, I would not expect Bitmap to necessarily keep the reference of the Stream object or filename string that was passed in, but the data they point to.
Of course a constructor and a function is the same internally. The entirety of programming language constructs reduces to gotos, after all. The purpose is to understandably convey ideas to humans, and in the case of an object constructor, the message is "this object consists of this data", while the message of a parse function is "this text represents this value".These are very different things semantically. Are you going to tell me next that every "ToString" method has no "real distinction" from a constructor? You'll notice that although many things can be converted to strings, the only constructors of the String class accept characters. Why? Because a string consists of characters.
Constructors imply construction, not conversion.
There is quite a difference between "might not behave that way" and "expected to behave the opposite of that way". What I am disputing is the latter.Admin
That second method doesn't actually work. That's why you see the first method almost everywhere.
Admin
new Bitmap(fileName).Save(HttpContext.Current.Response.OutputStream, ImageFormat.Jpeg);
or
new FileStream(filename, FileMode.Open).CopyTo(HttpContext.Current.Response.OutputStream);
Admin
While it may require some extra steps, once you've verified that the picture exists, that it's really a picture, and that the user has permission to see it, the simplest way to handle it is to send a content-type header and then just copy the image file to the output.